[OpenSER-Devel] postgresql and escaping

[Henning Westerholt]

On Thursday 24 January 2008, Klaus Darilion wrote:
> 1.
>      case DB_BLOB:
>          l = VAL_BLOB(_v).len;
>          if (*_len < (l * 2 + 3)) {
>              LM_ERR("destination buffer too short for blob\n");
>              return -7;
>          } else {
>              *_s++ = '\'';
>              tmp_s = (char*)PQescapeByteaConn(CON_CONNECTION(_con),
> (unsigned char*)VAL_STRING(_v),
>                      (size_t)l, (size_t*)&tmp_len);
>              if(tmp_s==NULL)
>              {
>                  LM_ERR("PQescapeBytea failed\n");
>                  return -7;
>              }
>              memcpy(_s, tmp_s, tmp_len);
>              PQfreemem(tmp_s);
>              tmp_len = strlen(_s);
>              *(_s + tmp_len) = '\'';
>              *(_s + tmp_len + 1) = '\0';
>              *_len = tmp_len + 2;
>              return 0;
>          }
>          break;
>
> This means we reserve l*2+3 bytes for the escaped string, but as
> escaping of special characters is done by 3digts octal representation,
> e.g. CR will be converted to \015, this buffer can be too small.
>
> I suggest to make the length check after PQescapeByteaConn and check if
> tmp_len < _len.

Hi Klaus,

sounds resonable, potential buffer overflows should be fixed. ;-). 
Do you have already created a fix that can be commited?

> 2. With postgresql 8.1 the handling of string escaping was changed
> (http://www.postgresql.org/docs/8.1/interactive/release-8-1.html). They
> introduced the E'' syntax and current escaping with \ will be obsolete.
> Thus, maybe we have to update the code to check server version we are
> connected too and thus use the proper escaping.

When will the 'old' method of escaping finally be removed? According to the 
documentation you've given above, there exists a variable that can be checked 
to get the supported escape method. Thus it should be no so hard to add some 
code in the escaping path to support this change.

Cheers,

Henning