[img] heap-buffer-overflow reading a v8 .3d with a non-default separator and no coordinate system

[Philip Schuchardt]

dump3d on the .3d cavern writes for tests/wallsdiving.srv aborts under AddressSanitizer:

==33594==ERROR: AddressSanitizer: heap-buffer-overflow READ of size 6 at 0x602000000120 thread T0

1 memcmp

2 img_read_stream_survey img.c:1400

3 img_for_survex_open_survey img_for_survex.c:46

4 main dump3d.c:92

0x602000000120 is located 0 bytes after 16-byte region allocated by getline_alloc_len img.c:555 from img_read_stream_survey img.c:1387

The title line of that file is "wallsdiving\0\0:" - the writer (img.c:1687) adds the hidden fields whenever the separator isn't '.', which is the case for every Walls import, so the cs field is an empty string. The reader then does

if (memcmp(cs, "+init=", 6) == 0) {

on it, which reads 6 bytes from a 0 length string. The line fits in getline_alloc_len's initial 16 byte buffer, so that runs 2 bytes past the end of the allocation. A .svx with no *cs and the default separator doesn't trip it because the extra fields aren't written at all.

Same shape one level down: with a cs of "+init=e" the p[4] / p[5] checks and the memcmp against "epsg"/"esri" read past the end too.

Seen with the clang ASan build on macOS arm64; the sanitisers CI job may not be reaching dump3d on the Walls tests. Unrelated to #150, found while running the full cavern.tst for that patch.

[Philip Schuchardt]

Fix for this issue

[Philip Schuchardt]

Patch attached!

[Olly Betts]

Thanks, except:

tests/wallsdiving.srv already triggers this: running dump3d on the .3d

cavern writes for it reports a heap-buffer-overflow under AddressSanitizer, so the sanitisers and valgrind CI jobs cover the fix.

Both those jobs are green before your fix though (so if your fix later got reverted CI would still pass...)