JACoders / OpenJK

Community effort to maintain and improve Jedi Academy (SP & MP) + Jedi Outcast (SP only) released by Raven Software
GNU General Public License v2.0
2.03k stars 614 forks source link

Supposedly pressing tab in lugormod servers causes a crash. #151

Closed ensiform closed 11 years ago

ensiform commented 11 years ago

I saw it happen in rd-vanilla (no debug symbols).

"thedt" says he can get it in the cgame (modbase)

thedt commented 11 years ago

The details it crashes on random maps like say you connect to /connect lugor.net and you are on yavin1b or t2_rancor when pushing tab it crashes when using the openjk cgame ,but the raven cgame does fine.

Razish commented 11 years ago

Digging around, the issue arises around CG_LoadClientInfo being called on clientNum >= 32 (default MAX_CLIENTS) Jenova does some weird things, and ends up setting cgs.maxclients to 64 When you load the scoreboard, it will attempt to load models for clientNum 32 <-> 63 (specifically 34 in my tests) resulting in reading/writing invalid data.

// CG_LoadClientInfo
    clientNum = ci - cgs.clientinfo;
    if (clientNum < 0 || clientNum >= MAX_CLIENTS)
        clientNum = -1;

By the time it gets to calling CG_RegisterClientModelname, this is the data it has:

        ci-cgs.clientinfo 34
        clientNum -1
        ci->modelName "d"
        ci->infoValid -2147483648
        ci->saberName "Ä"
        ci->team 1144520704

I'm not quite sure why this doesn't happen on Raven's cgame - I'm actually fairly certain it does, because that server tries to set these cvars forcefully, which is protected in OpenJK:

WARNING: server is not allowed to set r_drawfog=0
WARNING: server is not allowed to set cl_motdstring=lugormod.com
WARNING: server is not allowed to set cl_allowdownload=1
WARNING: server is not allowed to set cl_timeout=600
WARNING: server is not allowed to set rate=90000
WARNING: server is not allowed to set snaps=30
WARNING: server is not allowed to set com_buildScript=0

Specifically cl_allowdownload, which would then attempt to download a 'fixed' cgamex86.dll where MAX_CLIENTS is defined as 64 avoiding this issue.

I'm going to label this is as wontfix until we have more information. From what I can tell it's an issue of compatibility, or lack thereof. If Jenova Lugormod wants to do these fancy things, they're going to have to distribute a client modification, not rely on a security vulnerability to silently download their code.

Razish commented 11 years ago

Commit 1d3c97dfd49089216fbcfc13bcf38098727678a0 will prevent the crash from happening, not sure what side-effects this will have, not really a concern.

thedt commented 11 years ago

that seems to have fixed it thank you so much :D