Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signed integer overflow in JsonIn::get_int #36808

Closed
hexagonrecursion opened this issue Jan 8, 2020 · 1 comment · Fixed by #38453
Closed

Signed integer overflow in JsonIn::get_int #36808

hexagonrecursion opened this issue Jan 8, 2020 · 1 comment · Fixed by #38453
Labels
<Bug> This needs to be fixed

Comments

@hexagonrecursion
Copy link
Contributor

src/json.cpp:1063:41: runtime error: signed integer overflow: -2147483648 * -1 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/json.cpp:1063:41 in 
(lldb) bt
* thread #1, name = 'cataclysm-tiles', stop reason = Signed integer overflow
  * frame #0: 0x000000000216db50 cataclysm-tiles`__ubsan_on_report
    frame #1: 0x000000000216812d cataclysm-tiles`__ubsan::Diag::~Diag() + 669
    frame #2: 0x000000000216b32c cataclysm-tiles`void handleIntegerOverflowImpl<__ubsan::Value>(__ubsan::OverflowData*, unsigned long, char const*, __ubsan::Value, __ubsan::ReportOptions) + 620
    frame #3: 0x000000000216d1c3 cataclysm-tiles`__ubsan_handle_mul_overflow + 51
    frame #4: 0x0000000003f19111 cataclysm-tiles`JsonIn::get_int(this=<unavailable>) at json.cpp:1063:41
    frame #5: 0x0000000005951fc3 cataclysm-tiles`mission::deserialize(this=0x00007ffffffbb800, jsin=0x00007ffffffbacd0) at savegame_json.cpp:2815:23
    frame #6: 0x00000000058c837b cataclysm-tiles`mission::unserialize_all(jsin=0x00007ffffffbba80) at savegame.cpp:1592:13
    frame #7: 0x00000000058c8719 cataclysm-tiles`game::unserialize_master(this=0x0000619000158180, fin=<unavailable>) at savegame.cpp:1617:17
    frame #8: 0x000000000352aa49 cataclysm-tiles`void std::_Bind<void (game::* (game*, std::_Placeholder<1>))(std::istream&)>::__call<void, std::istream&, 0ul, 1ul>(this=0x000060300b4922c0, __args=0x00007ffffffbbdc0, (null)=<unavailable>) at functional:400:11
    frame #9: 0x000000000352a804 cataclysm-tiles`void std::_Bind<void (game::* (game*, std::_Placeholder<1>))(std::istream&)>::operator(this=0x000060300b4922c0, __args=<unavailable>)<std::istream&, void>(std::istream&) at functional:482:17
    frame #10: 0x00000000029a6a8b cataclysm-tiles`read_from_file(path="./save/Holabird/master.gsav", reader=0x00007ffffffbc2e0)> const&) at cata_utility.cpp:427:9
    frame #11: 0x000000000338e691 cataclysm-tiles`game::load_master(this=0x0000619000158180) at game.cpp:2627:5
    frame #12: 0x0000000003387fb7 cataclysm-tiles`game::start_game(this=0x0000619000158180) at game.cpp:705:5
    frame #13: 0x00000000041afb2a cataclysm-tiles`main_menu::new_character_tab(this=0x00007fffffffdb70) at main_menu.cpp:805:29
    frame #14: 0x00000000041ab41f cataclysm-tiles`main_menu::opening_screen(this=<unavailable>) at main_menu.cpp:539:37
    frame #15: 0x000000000418f5ba cataclysm-tiles`main(argc=<unavailable>, argv=0x00007fffffffe0c8) at main.cpp:683:23
    frame #16: 0x00007ffff78d5f43 libc.so.6`__libc_start_main(main=(cataclysm-tiles`main at main.cpp:135), argc=1, argv=0x00007fffffffe0c8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffe0b8) at libc-start.c:308:16
    frame #17: 0x0000000002054a6e cataclysm-tiles`_start + 46

Steps To Reproduce

Steps to reproduce the behavior:

  1. compile with clang UndefinedBehaviorSanitizer
  2. Start a new game via "play now (fixed scenario)"

Note that this is not 100% reliable and may take a couple of tries.

Expected behavior

Overflow of signed integer types in C++ should be avoided because it is undefined behavior.

Versions and configuration

  • OS: Linux
    • OS Version: Fedora 30
  • Game Version: 0.D-11025-g9fe2e72d43 [64-bit]
  • Graphics Version: Tiles
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food]
    ]
@Qrox
Copy link
Contributor

Qrox commented Jan 8, 2020

At json.cpp:1063

return static_cast<int>( n.number ) * ( n.negative ? -1 : 1 );

When n.number is 2147483648 it will trigger signed overflow.

This also happens in JsonIn::get_int64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants