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

Fix crash when the creature that kills the player no longer exists #60272

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Aug 18, 2022

Summary

None

Purpose of change

Followed the steps in the issue and got this output from ASAN:

==54496==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5555615a5fd8 at pc 0x55555a8511b7 bp 0x7fffffff2fb0 sp 0x7fffffff2fa0
READ of size 8 at 0x5555615a5fd8 thread T0
    #0 0x55555a8511b6 in get_talker_for(Creature*) src/creature.cpp:2982
    #1 0x55555ad782e4 in effect_on_conditions::avatar_death() src/effect_on_condition.cpp:391
    #2 0x55555b128967 in game::is_game_over() src/game.cpp:2604
    #3 0x55555abcd9ce in do_turn() src/do_turn.cpp:593
    #4 0x55555c3b5326 in main src/main.cpp:789
    #5 0x7ffff6e7d082 in __libc_start_main ../csu/libc-start.c:308
    #6 0x5555592398bd in _start (/home/dtsadmin/Builds/Cataclysm-DDA/cataclysm-tiles+0x3ce58bd)

0x5555615a5fd8 is located 8 bytes to the right of global variable 'typeinfo for Creature::auto_find_hostile_target(int, int&, int)::{lambda(Creature const&)#1}' defined in '/usr/include/c++/9/bits/std_function.h:203:48' (0x5555615a5fc0) of size 16
SUMMARY: AddressSanitizer: global-buffer-overflow src/creature.cpp:2982 in get_talker_for(Creature*)
Shadow bytes around the buggy address:
  0x0aab2c2acba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0aab2c2acbb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0aab2c2acbc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0aab2c2acbd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0aab2c2acbe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0aab2c2acbf0: 00 00 00 00 00 00 00 00 00 00 f9[f9]f9 f9 f9 f9
  0x0aab2c2acc00: 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9
  0x0aab2c2acc10: 00 00 00 f9 f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9
  0x0aab2c2acc20: 00 f9 f9 f9 f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9
  0x0aab2c2acc30: 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x0aab2c2acc40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==54496==ABORTING

Describe the solution

Check that the Creature pointer stored in Creature::killer still exists in the creature tracker before using it.

Describe alternatives you've considered

Testing

Following the steps in the issue no longer crashes the game, and the AVATAR_DEATH EOCs run without a "killer creature".

Also tested with a killer creature that doesn't disappear when the player dies (like a zombie hulk), and the AVATAR_DEATH EOCs run with that creature as the "killer".

(AFAIK, the killer creature isn't used by any of these EOCs currently)

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` EOC: Effects On Condition Anything concerning Effects On Condition astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Aug 18, 2022
@NetSysFire NetSysFire added Monsters Monsters both friendly and unfriendly. <Bugfix> This is a fix for a bug (or closes open issue) labels Aug 18, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 18, 2022
@bombasticSlacks bombasticSlacks merged commit 69afab5 into CleverRaven:master Aug 19, 2022
@dseguin dseguin deleted the fix_death_overflow branch August 19, 2022 22:11
pjf added a commit to pjf/Cataclysm-DDA that referenced this pull request Aug 21, 2022
* origin/master:
  Prepper NPC quest follow-up (CleverRaven#59515)
  Adds a new widget for th structured sidebar with all midsize bodygraphs (i.e. the ones structured uses for "Health + Status" et al) side by side with nothing else, for use with Health + Overmap because I like the way the midsize bodygraphs look and don't want to have to choose just one of them to display (CleverRaven#60244)
  [DinoMod] slower zombie dino upgrades (CleverRaven#60311)
  [Xedra Evolved] Adds itemgroups for spell items (CleverRaven#60296)
  Update mutations.json (CleverRaven#60295)
  Fix typos (CleverRaven#60269)
  Wooden pulley fixes (CleverRaven#60285)
  Update landscaping.json (CleverRaven#60299)
  Update martialarts.json (CleverRaven#60305)
  fix: 'person' monster silently vanishes after touching character (CleverRaven#60270)
  fix: prevent use of non valid creature pointer (CleverRaven#60272)
  Nested Recipes - Ropes, Bundles, Shelled Nuts (CleverRaven#60297)
  Nested Recipes - Chain Armor (CleverRaven#60268)
  Nerf isherwood dandelion quest rewards (CleverRaven#60273)
  Animal ports from TropiCata to Mainline + egg density fixes for insects and birds (CleverRaven#60234)
  Modular Defense Anchor is SOFT (CleverRaven#60279)
Hirmuolio pushed a commit to Hirmuolio/Cataclysm-DDA that referenced this pull request Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` EOC: Effects On Condition Anything concerning Effects On Condition json-styled JSON lint passed, label assigned by github actions Monsters Monsters both friendly and unfriendly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Being killed by a Bombardier Boomer crashes the game.
3 participants