Tile.cpp registered_tiles array bounds indexing out of range #29

New Issue

rich · 2025-01-16T09:10:47-05:00

rich commented

2025-01-16 09:10:47 -05:00

I did a build of ReCaveGame with addition compiler flags of -g -fsanitize=address -fno-omit-frame-pointer

This highlighted the following issues:

rich@jam:~/Redacted/ReCaveGame/build/ClientApp$ ./CaveClientApp
=================================================================
==10971==ERROR: AddressSanitizer: global-buffer-overflow on address 0x742c8b93ccd8 at pc 0x742c8b7822c5 bp 0x7ffd48cc9a70 sp 0x7ffd48cc9a60
WRITE of size 8 at 0x742c8b93ccd8 thread T0
    #0 0x742c8b7822c4 in CaveGame::Core::RegisterTile(CaveGame::Core::Tile*) /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:293
    #1 0x742c8b781f20 in CaveGame::Core::Tile::Tile(CaveGame::Core::TileID, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Color4 const&) /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:268
    #2 0x742c8b6e6de5 in __static_initialization_and_destruction_0 /home/rich/Redacted/ReCaveGame/Core/include/Core/Tile.hpp:333
    #3 0x742c8b6ec859 in _GLOBAL__sub_I_Chunk.cpp /home/rich/Redacted/ReCaveGame/Core/src/Core/Chunk.cpp:101
    #4 0x742c8c60f71e in call_init elf/dl-init.c:74
    #5 0x742c8c60f823 in call_init elf/dl-init.c:120
    #6 0x742c8c60f823 in _dl_init elf/dl-init.c:121
    #7 0x742c8c62959f  (/lib64/ld-linux-x86-64.so.2+0x1f59f) (BuildId: 353e1b6cb0eebc08cf3ff812eae8a51b4efd684e)

0x742c8b93ccd8 is located 0 bytes after global variable 'registered_tiles' defined in '/home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:6:30' (0x742c8b8bcce0) of size 524280
SUMMARY: AddressSanitizer: global-buffer-overflow /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:293 in CaveGame::Core::RegisterTile(CaveGame::Core::Tile*)
Shadow bytes around the buggy address:
  0x742c8b93ca00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x742c8b93ca80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x742c8b93cb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x742c8b93cb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x742c8b93cc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x742c8b93cc80: 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9 f9
  0x742c8b93cd00: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x742c8b93cd80: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x742c8b93ce00: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x742c8b93ce80: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x742c8b93cf00: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10971==ABORTING

I can fix this with changing line 6 of Tiles.cpp from:

std::array<Tile*, 65535> registered_tiles{};

To:

std::array<Tile*, 65536> registered_tiles{};

But not sure if that is the fix you want.

I did a build of ReCaveGame with addition compiler flags of -g -fsanitize=address -fno-omit-frame-pointer This highlighted the following issues: ``` rich@jam:~/Redacted/ReCaveGame/build/ClientApp$ ./CaveClientApp ================================================================= ==10971==ERROR: AddressSanitizer: global-buffer-overflow on address 0x742c8b93ccd8 at pc 0x742c8b7822c5 bp 0x7ffd48cc9a70 sp 0x7ffd48cc9a60 WRITE of size 8 at 0x742c8b93ccd8 thread T0 #0 0x742c8b7822c4 in CaveGame::Core::RegisterTile(CaveGame::Core::Tile*) /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:293 #1 0x742c8b781f20 in CaveGame::Core::Tile::Tile(CaveGame::Core::TileID, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Color4 const&) /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:268 #2 0x742c8b6e6de5 in __static_initialization_and_destruction_0 /home/rich/Redacted/ReCaveGame/Core/include/Core/Tile.hpp:333 #3 0x742c8b6ec859 in _GLOBAL__sub_I_Chunk.cpp /home/rich/Redacted/ReCaveGame/Core/src/Core/Chunk.cpp:101 #4 0x742c8c60f71e in call_init elf/dl-init.c:74 #5 0x742c8c60f823 in call_init elf/dl-init.c:120 #6 0x742c8c60f823 in _dl_init elf/dl-init.c:121 #7 0x742c8c62959f (/lib64/ld-linux-x86-64.so.2+0x1f59f) (BuildId: 353e1b6cb0eebc08cf3ff812eae8a51b4efd684e) 0x742c8b93ccd8 is located 0 bytes after global variable 'registered_tiles' defined in '/home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:6:30' (0x742c8b8bcce0) of size 524280 SUMMARY: AddressSanitizer: global-buffer-overflow /home/rich/Redacted/ReCaveGame/Core/src/Core/Tile.cpp:293 in CaveGame::Core::RegisterTile(CaveGame::Core::Tile*) Shadow bytes around the buggy address: 0x742c8b93ca00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x742c8b93ca80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x742c8b93cb00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x742c8b93cb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x742c8b93cc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x742c8b93cc80: 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9 f9 0x742c8b93cd00: 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x742c8b93cd80: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x742c8b93ce00: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x742c8b93ce80: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 0x742c8b93cf00: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==10971==ABORTING ``` I can fix this with changing line 6 of Tiles.cpp from: ``` std::array<Tile*, 65535> registered_tiles{}; ``` To: ``` std::array<Tile*, 65536> registered_tiles{}; ``` But not sure if that is the fix you want.

maxine commented

2025-01-16 09:54:18 -05:00

I'm surprised I didn't catch that. He made the array size max uint16 since that's how many tiles the game supports right now. Must have forgot the array size needs to be one larger for it to allocate all the elements.