Heretic: Segfault in rooms3.wad about chocolate-doom HOT 7 CLOSED

Invalid read of size 4
   at 0x14B115: PTR_AimTraverse (p_map.c:1202)
   by 0x14D42C: P_TraverseIntercepts (p_maputl.c:643)
   by 0x14C937: P_AimLineAttack (p_map.c:1407)
   by 0x14F8E6: P_SpawnPlayerMissile (p_mobj.c:1502)
   by 0x151320: A_FireSkullRodPL1 (p_pspr.c:1420)
   by 0x14FD11: P_SetPsprite (p_pspr.c:342)
   by 0x1500E2: P_FireWeapon (p_pspr.c:549)
   by 0x1500E2: P_FireWeapon (p_pspr.c:535)
   by 0x14FD11: P_SetPsprite (p_pspr.c:342)
   by 0x151C4D: P_MovePsprites (p_pspr.c:1900)
   by 0x15AA99: P_PlayerThink (p_user.c:646)
   by 0x1598A7: P_Ticker (p_tick.c:153)
   by 0x14101C: G_Ticker (g_game.c:1165)
 Address 0x0 is not stack'd, malloc'd or (recently) free'd

(gdb) insp *li
$2 = {v1 = 0x7ffff5520130, v2 = 0x7ffff5520140, dx = 0, dy = -4980736, flags = 260, special = 1, tag = 0, sidenum = {155, -1}, bbox = {65011712, 60030976, 14155776, 14155776}, slopetype = ST_VERTICAL, frontsector = 0x7ffff5521a00, 
  backsector = 0x0, validcount = 8505, specialdata = 0x0}

backsector is NULL. Is this linedef valid?

from chocolate-doom.

I did a little experiment. With modern Open Watcom and DOS4G extender a NULL pointer dereference actually works. Assuming it's mostly similar to the original Watcom the code would have worked in DOS and just read some random bytes.

The proper fix then will be to check the pointer against NULL and do something non-crashy when it is.

from chocolate-doom.

This and this additions will fix the issue. Seems to be demo-safe, at least I have no desyncs in longest full episode runs from DSDA demo archive.

Edit: yes, it should be safe, DSDA-Doom have similar fix.

from chocolate-doom.

This and this additions will fix the issue. Seems to be demo-safe, at least I have no desyncs in longest full episode runs from DSDA demo archive.

Edit: yes, it should be safe, DSDA-Doom have similar fix.

Could we have a PR, please?

from chocolate-doom.

Sure, if you say. My only thought is - this small corrections will need a comment(s), since such check is not included in original source release. But how such comment must look then? I mean, I shouldn't be something like "this fixes crash on modern OSes".

from chocolate-doom.

You could just reuse the relevant part of the original commit message for the Doom code:

d610772

from chocolate-doom.

Fixed in #1670. Thanks!

from chocolate-doom.