stack-buffer-overflow in zip filesystem

[GunChleoc]

To reproduce:

1. Drop the attached savefile into the save directory
2. Single Player -> Load Game

The bug is triggered by gl.preload_game(gpdp); in LoadOrSaveGame::fill_table()

=================================================================
==26734==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffcba31ea20 at pc 0x7fd0de8ad20b bp 0x7ffcba31e710 sp 0x7ffcba31deb8
READ of size 257 at 0x7ffcba31ea20 thread T0
    #0 0x7fd0de8ad20a in __interceptor_strlen (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x7020a)
    #1 0x7fd0dc85ac36 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x121c36)
    #2 0x186f7f8 in ZipFilesystem::file_exists(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/bratzbert/sources/widelands/trunk/src/io/filesystem/zip_filesystem.cc:223
    #3 0x18715dc in ZipFilesystem::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned long&) /home/bratzbert/sources/widelands/trunk/src/io/filesystem/zip_filesystem.cc:362
    #4 0x187483a in FileRead::open(FileSystem&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/bratzbert/sources/widelands/trunk/src/io/fileread.cc:33
    #5 0x184eedc in Profile::read(char const*, char const*, FileSystem&) /home/bratzbert/sources/widelands/trunk/src/profile/profile.cc:640
    #6 0x16abadf in Widelands::GamePreloadPacket::read(FileSystem&, Widelands::Game&, Widelands::MapObjectLoader*) /home/bratzbert/sources/widelands/trunk/src/game_io/game_preload_packet.cc:59
    #7 0x16a448f in Widelands::GameLoader::preload_game(Widelands::GamePreloadPacket&) /home/bratzbert/sources/widelands/trunk/src/game_io/game_loader.cc:57
    #8 0x1544fd5 in LoadOrSaveGame::fill_table() /home/bratzbert/sources/widelands/trunk/src/wui/load_or_save_game.cc:346
    #9 0x15422d7 in LoadOrSaveGame::LoadOrSaveGame(UI::Panel*, Widelands::Game&, LoadOrSaveGame::FileType, GameDetails::Style, bool) /home/bratzbert/sources/widelands/trunk/src/wui/load_or_save_game.cc:138
    #10 0x131c4d0 in FullscreenMenuLoadGame::FullscreenMenuLoadGame(Widelands::Game&, GameSettingsProvider*, bool) /home/bratzbert/sources/widelands/trunk/src/ui_fsmenu/loadgame.cc:51
    #11 0xda62ff in WLApplication::load_game() /home/bratzbert/sources/widelands/trunk/src/wlapplication.cc:1297
    #12 0xda4bb0 in WLApplication::mainmenu_singleplayer() /home/bratzbert/sources/widelands/trunk/src/wlapplication.cc:1136
    #13 0xda4162 in WLApplication::mainmenu() /home/bratzbert/sources/widelands/trunk/src/wlapplication.cc:1037
    #14 0xd9eaa2 in WLApplication::run() /home/bratzbert/sources/widelands/trunk/src/wlapplication.cc:439
    #15 0xd9bb00 in main /home/bratzbert/sources/widelands/trunk/src/main.cc:49
    #16 0x7fd0dbe7082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #17 0xd9b9c8 in _start (/home/bratzbert/sources/widelands/trunk/widelands+0xd9b9c8)

Address 0x7ffcba31ea20 is located in stack of thread T0 at offset 672 in frame
    #0 0x186f4f7 in ZipFilesystem::file_exists(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/bratzbert/sources/widelands/trunk/src/io/filesystem/zip_filesystem.cc:200

  This frame has 5 object(s):
    [32, 168) 'file_info'
    [224, 256) 'path_in'
    [288, 320) 'complete_filename'
    [352, 384) '<unknown>'
    [416, 672) 'filename_inzip' <== Memory access at offset 672 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)

[kaputtnik (franku)]

While i can unzip other save games, the attached save game couldn't be unzipped, because its broken. When using unzip in terminal:

unzip -l break_zip.wgf
Archive: break_zip.wgf
error [break_zip.wgf]: start of central directory not found;
  zipfile corrupt.
  (please check that you have transferred or created the zipfile in the
  appropriate BINARY mode and that you have compiled UnZip properly)

How was this save game created?

[kaputtnik (franku)]

For completeness, attached the output of unzip of a save game from Dezember 16. 2017.

[GunChleoc (gunchleoc)]

It's an automatically generated multiplayer savegame.

[Teppo Mäenpää (kxq)]

In my savegame directory contains hundreds of wgf files, one of which was corrupted. Is this really a critical problem?

[kaputtnik (franku)]

I think the problem here is a corrupted save game (zip-file). I can't check what happens if such a save game get loaded, because the provided one is incompatible now. From what i remember we had some issues, mostly on windows OS, where corrupted save games are created and couldn't be loaded anymore, e.g. bug 1654897. Not sure if this is the same reason. But, IMHO, there should be a check some where. E.g.:

After creating a save game, check if the resulting zip is valid. If not, either show a message to the user "Sorry, the produced save game is probably corrupt. Please try again to save." or retry automatically to create a save game. If possible a combined thing:

if save_game_invalid
    If autosave and creating_auto_save_game_failed > 1
    show_message()
    else
    show_message()

[GunChleoc (gunchleoc)]

I always classify crashes as critical at first. We should find a way to show a message to the user rather than crashing.

[GunChleoc (gunchleoc)]

Attaching a corrupt singleplayer game.

[GunChleoc (gunchleoc)]

Fixed in build20-rc1