Out of array bounds access in PM_AnimLength

[xycaleth]

Found using OSX 10.9, x64 debug build, but this looks to be a platform independent problem.

The problem is in NPC_ChoosePainAnimation - under some circumstances, the anim given to PM_AnimLength is -1 (in the stack trace, the number is unsigned). anim is then used to index into an array. The same problems exists in MP, though I haven't come across it yet. The following backtrace is from SP, but it looks like the problem would occur in MP as well:

(gdb) bt
#0  0x0000000111e2bd0e in PM_AnimLength (index=0, anim=4294967295) at /Users/alex/Documents/Programming/OpenJK/code/game/bg_panimate.cpp:4378
#1  0x0000000111d9671e in NPC_ChoosePainAnimation (self=0x11207e118, other=0x112085bd0, point=0x112085d2c, damage=98, mod=41, hitLoc=0, voiceEvent=-1) at /Users/alex/Documents/Programming/OpenJK/code/game/NPC_reactions.cpp:377
#2  0x0000000111d96e84 in NPC_Pain (self=0x11207e118, inflictor=0x112085bd0, other=0x112085bd0, point=0x112085d2c, damage=98, mod=41, hitLoc=0) at /Users/alex/Documents/Programming/OpenJK/code/game/NPC_reactions.cpp:542
#3  0x0000000111c95ce4 in NPC_ST_Pain (self=0x11207e118, inflictor=0x112085bd0, other=0x112085bd0, point=0x112085d2c, damage=98, mod=41, hitLoc=0) at /Users/alex/Documents/Programming/OpenJK/code/game/AI_Stormtrooper.cpp:345
#4  0x0000000111ce5281 in GEntity_PainFunc (self=0x11207e118, inflictor=0x112085bd0, attacker=0x112085bd0, point=0x112085d2c, damage=98, mod=41, hitLoc=0) at /Users/alex/Documents/Programming/OpenJK/code/game/g_functions.cpp:350
#5  0x0000000111cdf54e in G_Damage (targ=0x11207e118, inflictor=0x112085bd0, attacker=0x112085bd0, dir=0x7fff5a26b350, point=0x112085d2c, damage=98, dflags=66, mod=41, hitLoc=0) at /Users/alex/Documents/Programming/OpenJK/code/game/g_combat.cpp:6809
#6  0x0000000111ca7f2c in DoImpact (self=0x112085bd0, other=0x11207e118, damageSelf=1, trace=0x7fff5a26b480) at /Users/alex/Documents/Programming/OpenJK/code/game/g_active.cpp:1157
#7  0x0000000111e3ddf7 in PM_ClientImpact (trace=0x7fff5a26b480, damageSelf=1) at /Users/alex/Documents/Programming/OpenJK/code/game/bg_pmove.cpp:448
#8  0x0000000111e68c8c in PM_SlideMove (gravMod=1) at /Users/alex/Documents/Programming/OpenJK/code/game/bg_slidemove.cpp:193
#9  0x0000000111e694e1 in PM_StepSlideMove (gravMod=1) at /Users/alex/Documents/Programming/OpenJK/code/game/bg_slidemove.cpp:352
#10 0x0000000111e57a37 in PM_AirMove () at /Users/alex/Documents/Programming/OpenJK/code/game/bg_pmove.cpp:3011
#11 0x0000000111e53de6 in Pmove (pmove=0x7fff5a26c108) at /Users/alex/Documents/Programming/OpenJK/code/game/bg_pmove.cpp:14957
#12 0x0000000111cb66c2 in ClientThink_real (ent=0x112085bd0, ucmd=0x112427328) at /Users/alex/Documents/Programming/OpenJK/code/game/g_active.cpp:5460
#13 0x0000000111cb7431 in ClientThink (clientNum=206, ucmd=0x112427328) at /Users/alex/Documents/Programming/OpenJK/code/game/g_active.cpp:5722
#14 0x0000000111d80a5a in NPC_ExecuteBState (self=0x112085bd0) at /Users/alex/Documents/Programming/OpenJK/code/game/NPC.cpp:2362
#15 0x0000000111d8173a in NPC_Think (self=0x112085bd0) at /Users/alex/Documents/Programming/OpenJK/code/game/NPC.cpp:2563
#16 0x0000000111ce3eb5 in GEntity_ThinkFunc (self=0x112085bd0) at /Users/alex/Documents/Programming/OpenJK/code/game/g_functions.cpp:63
#17 0x0000000111cf1b0b in G_RunThink (ent=0x112085bd0) at /Users/alex/Documents/Programming/OpenJK/code/game/g_main.cpp:1052
#18 0x0000000111cf136f in G_RunFrame (levelTime=105850) at /Users/alex/Documents/Programming/OpenJK/code/game/g_main.cpp:2034
#19 0x0000000105a38a11 in SV_Frame (msec=18, fractionMsec=0) at /Users/alex/Documents/Programming/OpenJK/code/server/sv_main.cpp:576
#20 0x0000000105a0153e in Com_Frame () at /Users/alex/Documents/Programming/OpenJK/code/qcommon/common.cpp:1395
#21 0x0000000105aa35e2 in main (argc=7, argv=0x7fff5a26d9e8) at /Users/alex/Documents/Programming/OpenJK/code/sys/sys_main.cpp:387

EDIT: It looks like this could happen when the NPC is currently knocked down. This block of code in NPC_ChoosePainAnimation suggests that the call to PM_AnimLength shouldn't even happen when the NPC is knocked down:

            if ( G_CheckForStrongAttackMomentum( self )                                                   
                || PM_SpinningAnim( self->client->ps.legsAnim )                                           
                || PM_SaberInSpecialAttack( self->client->ps.torsoAnim )                                  
                || PM_InKnockDown( &self->client->ps )                                                    
                || PM_RollingAnim( self->client->ps.legsAnim )                                            
                || (PM_FlippingAnim( self->client->ps.legsAnim )&&!PM_InCartwheel( self->client->ps.legsAnim )) )          
            {//strong attacks, rolls, knockdowns, flips and spins cannot be interrupted by pain           
            }  

[ensiform]

Isn't there a function that validates the input before array access?

[xycaleth]

Nope

[ensiform]

Is fixed in [SP] 26a6261 / [MP] 8c28cfc