Programming flaw in some Death Message mods may crash servers

This article is obsolete

This article is pretty much obsolete. It was released approximately 12/19/98/ It only applied to mods that used source code based on Quake 2 v3.05 (practically speaking, this covered all versions up to, but not including, Quake 2 v3.12).

The reason this article is obsolete, in techno-Quake-geek terms, is that idsoftware introduced a meansOfDeath variable which makes it easy to know how somebody died. Thus the check for weapons is no longer necessary.

But, if you want to see what the fuss was about, read on...

Short description

While testing ServObit 1.1, I found a BFB (Big Freakin' Bug) that crashes the Quake 2 server. This BFB is most likely in other Death Message modifications, not just ServObit. The BFB occurs in a check for the type of weapon that an attaker has. There are two ways of doing this; one way that many Death Message mod authors have used (including me) is correct in probably over 99% of the case, but that last 1% causes the crash. NOTE that this bug is pretty obscure and requires a deeper understanding of how the code works than many Quake 2 modders (including me) have - after all, we've only had the code for a few weeks.

Some Death Message mods used "the other way" to determine the weapon being used, and as such are not prone to the bug.

Symptoms of the Big Freakin' Bug

The bug appears to be triggered when two players kill each other simultaneously. Thus it can be reliably duplicated. In ServObit 1.0, and likely in other Death Message mods, one person's death message is printed, but not the other's, and the server crashes. The weapons used to produce this bug were a BFG and a rocket launcher (nice and quick ;-)

Update on extent of the BFB

12/21/97... The BFB (Big Freakin' Bug) has apparently appeared in other mods besides ServObit. I've gotten feedback from several people describing inexplicable server crashes when two people killed each other at the same time. Most of the crashes were with death message mods - some not ServObit - but another one was with a CTF server, although I imagine that uses some form of death messages as well.

Determining the extent of the Big Freakin' Bug

I am uncertain as to the extent of this BFB. It appears to be in the source code of many other Death Message mods, but I haven't had the time to adequately test these other mods. Please email me if you have encountered symptoms as described above.

For details on how a proposed fix to the BFB, see the Technical Details below.

Technical Details

[Updated 12/20/97, 1:30 P.M. EST]

The flaw occurs in ClientObituary (of course) if you try to determine the weapon by looking at the weapon that the attacker is holding. Sometimes, attacker->client->pers.weapon->pickup_name appears to be "undefined" and the server has a page access violation and crashes when it tries to access it. It appears that attacker->client->pers.weapon is NULL. Here's the debugger output from Visual C++ 4.0:

// - attacker->client->pers.weapon->pickup_name 0x0466003a ""
// CXX0030: Error: expression cannot be evaluated

I've seen about 5 or 6 other Death Message mods and most of them rely on checking the pickup_name of the attacker's weapon without testing if pers.weapon is NULL. Thus, servers that are running your mods may crash because of this bug.

I don't know much about most of the Quake code, but perhaps the pers.weapon is reset when a player dies; perhaps in the same frame, later on, the other player's death is recorded and the code looks at the original player's weapon, and it crashes. Evidence that supports this is that one player's Obituary is printed but not the other's.

Other evidence for this is in the function ChangeWeapon in p_weapon.c:

	if (!ent->client->pers.weapon)	{
		// dead
		ent->client->ps.gunindex = 0;
		return;
	}

A conditional of this sort also appears in Cmd_WeapNext_f of g_cmds.c and several other places. So this looks like the conditional to use for testing if you can look at the attacker's weapon.

The only places I see pers.weapon assigned to are in ChangeWeapon (p_weapon.c) and InitClientPersistant (p_client.c), but in the latter case it appears that the item assigned is the standard-issue blaster ;-)

ServObit 1.1 Solution Partially Tested

I have not had the time or resources to test this solution, but I wanted to get this report out as soon as possible. I suggest using the above code snippet to check for a NULL pers.weapon and if that's the case, look at the inflictor. However, that leaves another challenge for Death Message modders.

Dealing with incomplete inflictor information

Since we can't count on pers.weapon to be defined each and every time, we have to look elsewhere. My way of solving this problem is to look at the inflictor if the attacker's weapon is "undefined." It's a hack of sorts and weapon modders might not like the solution if they use the package, but I look at the inflictor damage which is (generally) different between weapons. However, there's still a risk because, for example, chainguns and super shotguns inflict the same amount of damage.

You can't rely on the classnames of the inflictors, because, for example, a blast has the same classname whether it comes from a HyperBlaster or a regular blaster, and rockets don't have any classnames at all. I really don't want to rely on things like radius damage and inflictor damage because these are likely to be changed by people who modify weapons and therefore could affect selection of the proper obituary, but that's what I've decided to do for ServObit 1.1 - at least partially.

If the inflictor can't identify the proper weapon, then it goes to look at the attacker's weapon, if it exists. Hopefully this will cover all the bases; but in an extremely rare situation, it might not be known which weapon was fired. In this case I can rely on ServObit's wildcard capabilities to catch such a situation.

A more concrete solution is to add a new field to the player type that records the "lastweapon" that hit them or somesuch; but, this would require changing code in other source files, possibly increasing the complexity of the mod and making it a little more difficult for other modifiers who include a death message package that takes this approach.

Suggestions From Others

I've gotten some technical suggestions for solutions from James Abbatiello and Christian Wilson that both take a similar approach by more completely identifying the inflictor type in other places in the source code. As I said, I'm trying to stay away from this "patch the original code" approach because I'm concerned about ease of integration of ServObit with other mods. Hopefully idsoftware will more cleanly address this problem in the point release of Quake 2.

If you have other feedback, please email me. Thanks.

Email SteQve about the BFB.

Back to SteQve's No Frills Quake Mods Page

This page has been accessed times. It was last updated 12/20/97.