Re: may be a buffer overflow problem

> On 14 Jun 2024, at 17:18, Tom Lane  wrote:
> 
> I wrote:
>> Seeing that this code is exercised thousands of times a day in the
>> regression tests and has had a failure rate of exactly zero (and
>> yes, the tests do check the output), there must be some reason
>> why it's okay.
> 
> After looking a little closer, I think the reason why it works in
> practice is that there's always a few bytes of zero padding at the
> end of struct sqlca_t.
> 
> I don't see any value in changing individual code sites that are
> depending on that, because there are surely many more, both in
> our own code and end users' code.  What I suggest we ought to do
> is formalize the existence of that zero pad.  Perhaps like this:
> 
> char sqlstate[5];
> + char sqlstatepad;     /* nul terminator for sqlstate */
> };
> 
> Another way could be to change
> 
> - char sqlstate[5];
> + char sqlstate[6];
> 
> but I fear that could have unforeseen consequences in code that
> is paying attention to sizeof(sqlstate).

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.

The attached adds padding and adjust the tests and documentation to match.  I
kept the fprintf using %.*s to match other callers.  I don't know ECPG well
enough to have strong feelings wrt this being the right fix or not, and the age
of incorrect assumptions arounf that fprintf hints at this not being a huge
problem in reality (should still be fixed of course).

--
Daniel Gustafsson