RE: [***DKIM violation***] Re: [PATCH] Fix heap-buffer-overflow in PGTYPEStimestamp_defmt_scan()

Hello!

I apologize for the delay in sending the update, as your previous message unfortunately got lost in my inbox.

I have updated the patch to fix MSVC compatibility by replacing the GNU-specific range extension with memset(), removed the printf() and added an explanatory comment for ASAN/Valgrind, expanded the test coverage to check all four fixed format tokens (%D, %r, %R, %T).

The updated patch is attached.

Best regards,
Maria Sivirilova

От: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Дата: вторник, 3 ноября 2026 г. в 13:01
Кому: Daniel Gustafsson <daniel(at)yesql(dot)se>
Копия: pgsql-hackers(at)lists(dot)postgresql(dot)org <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Сивирилова Мария Константиновна <m(dot)sivirilova(at)ftdata(dot)ru>
Тема: [***DKIM violation***] Re: [PATCH] Fix heap-buffer-overflow in PGTYPEStimestamp_defmt_scan()
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.