Re: may be a buffer overflow problem

> On 14 Jun 2024, at 17:18, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 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