Editor crash with automatic automapper on large layers

[Robyt3]

Reported by texnonik:

1. Create a new map.
2. Add an image that has automapper rules.
3. Add a tile layer to the game group.
4. Increase the tile layer size to 500x500.
5. Select the image and an automapper rule for the tile layer.
6. Change the automapper seed and enable automatic mode with the A-button.

DDNet.exe caused an Access Violation at location 00007FF73C7D3FA2 in module DDNet.exe Reading from location 00000148DE5D6000.

AddrPC           Params
00007FF73C7D3FA2 00000148C9BA6338 0000000000000000 43AFE38E434EC71C  DDNet.exe!CAutoMapper::ProceedLocalized+0x440  [src/game/editor/auto_map.cpp @ 426]
   424: const CTile *pInGame = &pGameLayer->m_pTiles[y * pGameLayer->m_Width + x];
   425: CTile *pOutGame = &pUpdateGame->m_pTiles[(y - UpdateFromY) * pUpdateGame->m_Width + x - UpdateFromX];
>  426: pOutGame->m_Index = pInGame->m_Index;
   427: pOutGame->m_Flags = pInGame->m_Flags;
   428: }
00007FF73C93BB41 00000148DA723C00 0000000000000000 0000001B00000000  DDNet.exe!CLayerTiles::FlagModified+0x109  [src/game/editor/mapitems/layer_tiles.cpp @ 1349]
  1347: if(m_Seed != 0 && m_AutoMapperConfig != -1 && m_AutoAutoMap && m_Image >= 0)
  1348: {
> 1349: m_pEditor->m_Map.m_vpImages[m_Image]->m_AutoMapper.ProceedLocalized(this, m_pEditor->m_Map.m_pGameLayer.get(), m_AutoMapperReference, m_AutoMapperConfig, m_Seed, x, y, w, h);
  1350: }
  1351: }
00007FF73C9399E7 0000001B040FEB10 00000148FFFFFFFF 0000001B00000000  DDNet.exe!CLayerTiles::RenderProperties+0x30f  [src/game/editor/mapitems/layer_tiles.cpp @ 988]
   986: {
   987: m_AutoAutoMap = !m_AutoAutoMap;
>  988: FlagModified(0, 0, m_Width, m_Height);
   989: if(!m_TilesHistory.empty()) // Sometimes pressing that button causes the automap to run so we should be able to undo that
   990: {
00007FF73C95AE88 0000001B040FEC60 0000000040800000 0000001B00000001  DDNet.exe!CEditor::PopupLayer+0xa58  [src/game/editor/popups.cpp @ 836]
   834: s_Tracker.End(Prop, State);
   835: 
>  836: return pCurrentLayer->RenderProperties(&View);
   837: }
   838: 
00007FF73C7C465B 00000148C9BA23C0 0000001B040FEDC0 0000001B040FEDA0  DDNet.exe!CUi::RenderPopupMenus+0x245  [src/game/client/ui.cpp @ 1629]
  1627: // The popup render function can open/close popups, which may resize the vector and thus
  1628: // invalidate the variable PopupMenu. We therefore store pId in a separate variable.
> 1629: EPopupMenuFunctionResult Result = PopupMenu.m_pfnFunc(PopupMenu.m_pContext, PopupRect, Active);
  1630: if(Result != POPUP_KEEP_OPEN || (Active && ConsumeHotkey(HOTKEY_ESCAPE)))
  1631: ClosePopupMenu(pId, Result == POPUP_CLOSE_CURRENT_AND_DESCENDANTS);
00007FF73C80BDAA 00000148C9BA23C0 43166E5CC452C0A7 0000001B040FEE50  DDNet.exe!CEditor::Render+0x182e  [src/game/editor/editor.cpp @ 8305]
  8303: }
  8304: 
> 8305: Ui()->RenderPopupMenus();
  8306: FreeDynamicPopupMenus();
  8307: 
00007FF73C810178 3F8000003F800000 0000000000000001 000000003B9ACA00  DDNet.exe!CEditor::OnRender+0x17c  [src/game/editor/editor.cpp @ 9099]
  9097: Ui()->Update(m_MouseWorldPos);
  9098: 
> 9099: Render();
  9100: 
  9101: m_MouseDeltaWorld = vec2(0.0f, 0.0f);

c52401b3567a7d723c7c10e38e209a26d361477f (19.2 RC)

For some reason I cannot reproduce this with latest master (72e98930a628422917d8e31c4014d410ea245d0a) even though there should be no relevant difference between them.

[def-]

This seems like a regression in a new feature in 19.2, so we should either fix it or disable that feature.

[ChillerDragon]

@def- any reason you assigned me here? Did you blame a commit of mine introducing the issue?

[def-]

I thought it might be related to https://github.com/ddnet/ddnet/pull/9443, but maybe not.

[KebsCS]

Can't reproduce

[Robyt3]

Can reproduce with ASAN:

=================================================================
==3408==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x626000068810 at pc 0x55555b3ee514 bp 0x7fffffffaff0 sp 0x7fffffffafe0
READ of size 1 at 0x626000068810 thread T0
    #0 0x55555b3ee513 in CAutoMapper::ProceedLocalized(CLayerTiles*, CLayerTiles*, int, int, int, int, int, int, int) ddnet/src/game/editor/auto_map.cpp:426
    #1 0x55555bbd1312 in CLayerTiles::FlagModified(int, int, int, int) ddnet/src/game/editor/mapitems/layer_tiles.cpp:1354
    #2 0x55555bbba8fb in CLayerTiles::RenderProperties(CUIRect*) ddnet/src/game/editor/mapitems/layer_tiles.cpp:993
    #3 0x55555bc9b250 in CEditor::PopupLayer(void*, CUIRect, bool) ddnet/src/game/editor/popups.cpp:836
    #4 0x55555b39b3d7 in CUi::RenderPopupMenus() ddnet/src/game/client/ui.cpp:1661
    #5 0x55555b598af4 in CEditor::Render() ddnet/src/game/editor/editor.cpp:8305
    #6 0x55555b5c9599 in CEditor::OnRender() ddnet/src/game/editor/editor.cpp:9099
    #7 0x55555a247ffd in CClient::Render() ddnet/src/engine/client/client.cpp:1059
    #8 0x55555a2ad6a1 in CClient::Run() ddnet/src/engine/client/client.cpp:3302
    #9 0x55555a318f61 in main ddnet/src/engine/client/client.cpp:4924
    #10 0x7ffff3829d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #11 0x7ffff3829e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #12 0x555559e2b604 in _start (ddnet/build-asan/DDNet+0x48d7604)

0x626000068810 is located 0 bytes to the right of 10000-byte region [0x626000066100,0x626000068810)
allocated by thread T0 here:
    #0 0x7ffff74b6357 in operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x55555bb684e1 in CLayerTiles::CLayerTiles(CEditor*, int, int) ddnet/src/game/editor/mapitems/layer_tiles.cpp:43
    #2 0x55555ba999e0 in CLayerGame::CLayerGame(CEditor*, int, int) ddnet/src/game/editor/mapitems/layer_game.cpp:9
    #3 0x55555bc3ebeb in void __gnu_cxx::new_allocator<CLayerGame>::construct<CLayerGame, CEditor*&, int, int>(CLayerGame*, CEditor*&, int&&, int&&) /usr/include/c++/11/ext/new_allocator.h:162
    #4 0x55555bc3e84f in void std::allocator_traits<std::allocator<CLayerGame> >::construct<CLayerGame, CEditor*&, int, int>(std::allocator<CLayerGame>&, CLayerGame*, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/alloc_traits.h:516
    #5 0x55555bc3e382 in std::_Sp_counted_ptr_inplace<CLayerGame, std::allocator<CLayerGame>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<CEditor*&, int, int>(std::allocator<CLayerGame>, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr_base.h:519
    #6 0x55555bc3d868 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<CLayerGame, std::allocator<CLayerGame>, CEditor*&, int, int>(CLayerGame*&, std::_Sp_alloc_shared_tag<std::allocator<CLayerGame> >, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr_base.h:650
    #7 0x55555bc3d4af in std::__shared_ptr<CLayerGame, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<CLayerGame>, CEditor*&, int, int>(std::_Sp_alloc_shared_tag<std::allocator<CLayerGame> >, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr_base.h:1342
    #8 0x55555bc3ce41 in std::shared_ptr<CLayerGame>::shared_ptr<std::allocator<CLayerGame>, CEditor*&, int, int>(std::_Sp_alloc_shared_tag<std::allocator<CLayerGame> >, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr.h:409
    #9 0x55555bc3c161 in std::shared_ptr<CLayerGame> std::allocate_shared<CLayerGame, std::allocator<CLayerGame>, CEditor*&, int, int>(std::allocator<CLayerGame> const&, CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr.h:863
    #10 0x55555bc3ba46 in std::shared_ptr<CLayerGame> std::make_shared<CLayerGame, CEditor*&, int, int>(CEditor*&, int&&, int&&) /usr/include/c++/11/bits/shared_ptr.h:879
    #11 0x55555bc368dd in CEditorMap::CreateDefault(IGraphics::CTextureHandle) ddnet/src/game/editor/mapitems/map.cpp:140
    #12 0x55555b5ada41 in CEditor::Reset(bool) ddnet/src/game/editor/editor.cpp:8673
    #13 0x55555b5c6677 in CEditor::OnUpdate() ddnet/src/game/editor/editor.cpp:9044
    #14 0x55555a29534b in CClient::Update() ddnet/src/engine/client/client.cpp:2948
    #15 0x55555a2aaa60 in CClient::Run() ddnet/src/engine/client/client.cpp:3253
    #16 0x55555a318f61 in main ddnet/src/engine/client/client.cpp:4924
    #17 0x7ffff3829d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow ddnet/src/game/editor/auto_map.cpp:426 in CAutoMapper::ProceedLocalized(CLayerTiles*, CLayerTiles*, int, int, int, int, int, int, int)
Shadow bytes around the buggy address:
  0x0c4c800050b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c800050c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c800050d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c800050e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4c800050f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4c80005100: 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4c80005110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4c80005120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4c80005130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4c80005140: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4c80005150: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Shadow gap:              cc
==3408==ABORTING