Re: may be a buffer overflow problem
От | Daniel Gustafsson |
---|---|
Тема | Re: may be a buffer overflow problem |
Дата | |
Msg-id | 1C5D29FB-61D2-44F2-AC97-7FF6B20F20BA@yesql.se обсуждение исходный текст |
Ответ на | Re: may be a buffer overflow problem (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: may be a buffer overflow problem
|
Список | pgsql-hackers |
> On 14 Jun 2024, at 17:18, Tom Lane <tgl@sss.pgh.pa.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
Вложения
В списке pgsql-hackers по дате отправления: