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 incorrect tracker_register length #8

Merged
merged 1 commit into from Aug 27, 2014
Merged

fix incorrect tracker_register length #8

merged 1 commit into from Aug 27, 2014

Conversation

adam2104
Copy link

The tracker register messages actually packs 15 bytes of data into the pBuf, however, only 14 bytes are allocated. This causes a SegV exception on OS X when attempting to host a network game.

@kiplingw
Copy link

Good eye, Adam. Thanks for sharing.

@Matt1360
Copy link
Contributor

Whoops. I'm surprised it took a few years to even hear of this.

Matt1360 added a commit that referenced this pull request Aug 27, 2014
fix incorrect tracker_register length
@Matt1360 Matt1360 merged commit bd353a1 into dxx-rebirth:unification/master Aug 27, 2014
@adam2104
Copy link
Author

I was surprised too. I couldn't figure out why D2 kept crashing on OS X when I would start the game. What's maybe more surprising is why Windows didn't seem to mind. I had actually found the issue originally when writing my backup Tracker in python. See here for more detail - http://www.dxx-rebirth.com/frm/index.php/topic,1929.0.html

In any case, thanks for accepting it.

@adam2104 adam2104 deleted the fix-register-len branch August 27, 2014 20:10
@vLKp
Copy link
Contributor

vLKp commented Aug 28, 2014

Compilers often pad arrays out to the next full machine word, so the effective size may have been 16 when built with gcc. I suspect adam used clang, which may have different semantics.

@technomancy technomancy mentioned this pull request Nov 28, 2014
vLKp added a commit that referenced this pull request Jun 20, 2015
Only respawn when the player presses a fire key/button (primary,
secondary, or flare).  Only clear primary, second, flare, and bomb when
respawning.  This allows dead players to use the automap or change their
camera views.

This is a necessary step to flushing only the firing inputs on respawn,
as requested by Mako88 in
<#104>.  Currently,
inputs are flushed again by the activation of the game window, so
respawn still flushes all inputs.

	#1  0x00005555555cee19 in game_flush_inputs () at similar/main/game.cpp:374
	#2  game_handler (event=...) at similar/main/game.cpp:1114
	#3  0x0000555555576113 in window_send_event (wind=..., event=...) at common/arch/sdl/window.cpp:208
	#4  0x000055555557627b in WINDOW_SEND_EVENT (file=0x5555556dc126 "common/arch/sdl/window.cpp", e=0x5555556dc141 "EVENT_WINDOW_ACTIVATED", line=179, event=..., w=...) at common/include/window.h:111
	#5  window_set_visible (w=..., visible=visible@entry=1) at common/arch/sdl/window.cpp:179
	#6  0x00005555555e90f5 in window_set_visible (visible=1, wind=<optimized out>) at common/include/window.h:90
	#7  DoPlayerDead () at similar/main/gameseq.cpp:1479
	#8  0x000055555563ed65 in dead_player_frame () at similar/main/object.cpp:1486
	#9  0x00005555555cf4df in GameProcessFrame () at similar/main/game.cpp:1329
	#10 game_handler (event=...) at similar/main/game.cpp:1157
@ghost ghost mentioned this pull request Oct 10, 2016
@ghost ghost mentioned this pull request Dec 27, 2016
plomari pushed a commit to plomari/dxx that referenced this pull request Jun 11, 2021
I got this in Fight For Your Life, level 19:

main/wall.c:314(wall_open_door): Assert(w->type == WALL_DOOR)
=================================================================
==3862770==ERROR: AddressSanitizer: global-buffer-overflow on address
0x562dee57e448 at pc 0x562decd2169c bp 0x7ffc5d624400 sp 0x7ffc5d6243f8
READ of size 2 at 0x562dee57e448 thread T0
    #0 0x562decd2169b in find_connect_side main/gameseg.c:85
    dxx-rebirth#1 0x562ded061683 in wall_open_door main/wall.c:367
    dxx-rebirth#2 0x562decc10ccf in collide_robot_and_wall main/collide.c:115
    dxx-rebirth#3 0x562decc3d5a1 in collide_object_with_wall main/collide.c:2598
    dxx-rebirth#4 0x562decfa4ef1 in do_physics_sim main/physics.c:529
    dxx-rebirth#5 0x562decf8f1c5 in object_move_one main/object.c:1727
    dxx-rebirth#6 0x562decf92000 in object_move_all main/object.c:1858
    dxx-rebirth#7 0x562deccc8090 in GameProcessFrame main/game.c:1126
    dxx-rebirth#8 0x562deccc5ea9 in game_handler main/game.c:904
    dxx-rebirth#9 0x562decb5c486 in window_send_event arch/sdl/window.c:230
    dxx-rebirth#10 0x562decb4ed2c in event_process arch/sdl/event.c:184
    dxx-rebirth#11 0x562decb4efa2 in window_run_event_loop arch/sdl/event.c:206
    dxx-rebirth#12 0x562decdfbe8d in main main/inferno.c:341
    dxx-rebirth#13 0x7f7ec936ed09 in __libc_start_main ../csu/libc-start.c:308
(/mnt/big/doh/sandgrube/dxx-rebirth/d2x-rebirth-gl+0x7bcd69)

0x562dee57e448 is located 4 bytes to the right of global variable
'd_tick_step' defined in 'main/mglobal.c:38:5' (0x562dee57e440) of size
4
0x562dee57e448 is located 56 bytes to the left of global variable
'Segments' defined in 'main/mglobal.c:40:9' (0x562dee57e480) of size
10720000
SUMMARY: AddressSanitizer: global-buffer-overflow main/gameseg.c:85 in
find_connect_side
Shadow bytes around the buggy address:
  0x0ac63dca7c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c70: 00 00 00 00 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
=>0x0ac63dca7c80: 04 f9 f9 f9 f9 f9 f9 f9 04[f9]f9 f9 f9 f9 f9 f9
  0x0ac63dca7c90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7ca0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cd0: 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
==3862770==ABORTING

Possibly unreproducable. Judging from collide_robot_and_wall, maybe it's
a case where a wall was marked as needing a key, but there's no door?
Unknown. Try to avoid this by returning if it makes no sense.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants