| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Sivirilova Marija <m(dot)sivirilova(at)ftdata(dot)ru> |
| Subject: | Re: [PATCH] Fix heap-buffer-overflow in PGTYPEStimestamp_defmt_scan() |
| Date: | 2026-08-19 21:56:28 |
| Message-ID: | CAN4CZFP7edXdvBoLVrdkFoyM7O1cRpWtcCEbNnpwW-Hg4JgGJw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello!
The fix itself looks good to me, I only seem some issues in the test.
--- a/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
+++ b/src/interfaces/ecpg/test/pgtypeslib/dt_test2.pgc
If the test produces additional output, you should also update the
output file. (but also see my later comment about this)
+ char large_fmt[10000] = {
+ [0] = '%',
+ [1] = 'D',
+ [2 ... 9998] = 'A'
+ };
+
2...9998 is a GNU extension, it won't compile on MSVC.
+ PGTYPEStimestamp_defmt_asc("1 minute", large_fmt, &ts1);
+ printf("Success parsing large string\n");
Won't this fail parsing? I am not saying that it wouldn't trigger an
asan/valgrind report when executed with that and the original code,
but that printed message is misleading. (it won't succeed ; we aren't
parsing a large string, it's a small string parsed with a large format
string). It also only tests 1 of the 4 fixed locations.
I probably wouldn't print anything for these tests, as the output
won't help deciding if the test failed or passed, it is always the
same. Maybe a comment explaining that these calls can trigger
asan/valgrind would be more useful?
> For anyone reviewing (I at least won't have time for a few days):
> there is also an earlier patch submitted for fixing this bug which should be
> considered to make sure we pick the best fix.
These seems to be similar but different issue to me.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-08-19 22:17:39 | Re: Fix PGTYPESdate_fmt_asc overflow when a year does not fit "yyyy" |
| Previous Message | Bruce Momjian | 2026-08-19 21:24:37 | Re: hashjoins vs. Bloom filters (yet again) |