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’ output truncated 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