Re: may be a buffer overflow problem

Hi,

On 2024-06-17 23:52:54 +0200, Daniel Gustafsson wrote:
> Since sqlca is, according to our docs, present in other database systems we
> should probably keep it a 5-char array for portability reasons.  Adding a
> padding character should be fine though.

How about, additionally, adding __attribute__((nonstring))? Wrapped in an
attribute, of course.  That'll trigger warning for many unsafe uses, like
strlen().

It doesn't quite detect the problematic case in ecpg_log() though, seems it
doesn't understand fprintf() well enough (it does trigger in simple printf()
cases, because they get reduced to puts(), which it understands).


Adding nonstring possibly allow us to re-enable -Wstringop-truncation, it triggers a
bunch on

../../../../../home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c: In function ‘ECPGset_var’:
../../../../../home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c:575:17: warning: ‘__builtin_strncpy’
outputtruncated before terminating nul copying 5 bytes from a string of the same length [-Wstringop-truncation]
 
  575 |                 strncpy(sqlca->sqlstate, "YE001", sizeof(sqlca->sqlstate));


The only other -Wstringop-truncation warnings are in ecpg tests and at least
the first one doesn't look bogus:

../../../../../home/andres/src/postgresql/src/interfaces/ecpg/test/compat_oracle/char_array.pgc: In function 'main':
../../../../../home/andres/src/postgresql/src/interfaces/ecpg/test/compat_oracle/char_array.pgc:54:5: warning:
'__builtin_strncpy'output truncated before terminating nul copying 5 bytes from a string of the same length
[-Wstringop-truncation]
   54 |     strncpy(shortstr, ppppp, sizeof shortstr);

Which seems like a valid complaint, given that shortstr is a char[5], ppppp
is "XXXXX" and thatshortstr is printed:
    printf("\"%s\": \"%s\"  %d\n", bigstr, shortstr, shstr_ind);


Greetings,

Andres Freund