A Stroke of Fate: Fixing a fourteen years old bug

23 minutes to read

As many other old games the one we’re going to look at here doesn’t run well on Windows 10. In fact, it becomes unplayable after a certain part. All because of a critical scripting error that’s been there since day one, but only surfaced many years later. Finding and fixing this bug involved writing disassembler and assembler for Wintermute Engine scripts.

Download fix: shake.script

To install just copy shake.script file into “<game_root>/scenes/bunker” overwriting existing one (make a backup just in case).

The “front-end” of a problem🔗

According to a visual appearance game crashes some undetermined time after entering a bunker either with a “pure virtual function call” exception message or with no message at all. This suggests that potentially there are multiple (and maybe unrelated) problems.

Game exception messagebox — A “pure virtual function call” message

Already it gives us good initial breaking points for debugging.

Debugging the issue🔗

Hooking up a debugger that’s able to handle a low-level code (I’ll be using x32dbg — a version of x64dbg to debug 32-bit applications) to the running game revealed an instruction that was causing an exception:

x64dbg disassembly window — Instruction causing game to crash

In both cases it was an indirect call into a virtual function through a virtual functions table (C++ virtual method call). Sometimes call address contained a jump to _purecall and sometimes garbage (in my case it was just zero, resulting in a memory access violation also known as segfault).

Further analysis gave information that a caller is probably a function executing virtual machine instructions. A class name of object passed into it as “this” pointer is CScScript (acquired through RTTI), it has large switch-case statement and error messages like "Script stack corruption detected <...>" and "Fatal: Invalid instruction <...>" nearby. Checking WMEs source code confirmed it.

It was a CScScript::ExecuteInstruction function and VM instruction in question is II_CALL_BY_EXP. Game crashed attempting to call CanHandleMethod virtual function on a VAL_NATIVE type value pointed to by CScValue object popped from a script stack:

case II_CALL_BY_EXP: {
  // push var
  // push string
  str = m_Stack->Pop()->GetString();
  char* MethodName = new char[strlen(str)+1];
  strcpy(MethodName, str);

  CScValue* var = m_Stack->Pop();
  if(var->m_Type==VAL_VARIABLE_REF) var = var->m_ValRef;

  HRESULT res = E_FAIL;
  // <...>
  if(FAILED(res)){
    // Crash caused by an attempt to call "CanHandleMethod"
    // through invalid vftable
    if(var->IsNative() && var->GetNative()->CanHandleMethod(MethodName)){
      // <...>
    } else{
      // <...>
    }
  }
  delete [] MethodName;
}
break;

// <...>

bool CScValue::IsNative()
{
  if(m_Type==VAL_VARIABLE_REF) return m_ValRef->IsNative();
  return (m_Type == VAL_NATIVE);
}

CBScriptable* CScValue::GetNative()
{
  if(m_Type==VAL_VARIABLE_REF) return m_ValRef->GetNative();
  if(m_Type==VAL_NATIVE) return m_ValNative;
  else return NULL;
}

It appears that this “native value” m_ValNative is getting corrupted somehow. What’s interesting here is that it doesn’t seem to be the case on OSes before Windows 8. Testing on my Windows 7 machine proved it to be true. No crashes there. This suggests the problem is most likely related to dynamically linked libraries (i.e. DLLs) that comes outside of game folder — these might be of various versions and behave differently.

The next logical step is to figure out how this “native value” is created and pushed on stack. For this we would have to disassemble a script that contains this problematic II_CALL_BY_EXP instruction, therefore locate and extract it first.

Dumping a script file🔗

Knowing that we’re in CScScript member function we can extract its fields through “this” pointer which is stored in ebp register (how peculiar). Referencing the source code it’s easy to figure out offsets of specific class members:

x64dbg memory dump window — `CScScript` annotated memory dump

The m_Stack contains a pointer to script stack object, m_CurrentLine is the current source code line set by II_DBG_LINE instruction, m_IP is an offset of currently executing instruction, m_BufferSize is the size of a buffer that stores a script file, m_Buffer contains an entire script file and m_Filename is the script file name (also stored in a script file).

Filename of a script we’re interested in is scenes\bunker\shake.script. Problematic instruction offset within a script file is 00002B37. Dumping in this case is done by simply reading m_BufferSize bytes at m_Buffer address and storing it in a file on disk.

Looking at the script file🔗

Now that we have extracted a possibly buggy script file it’s time to look what’s going on inside. We need a disassembler for that. Unable to find existing WME script disassemblers it was time to write one.

Basics of WME virtual machine🔗

Wintermute’s virtual machine is stack based, there are no registers that can be directly accessed. Stack grows in a positive direction starting from -1 initial value, thus top of a stack (if stack is not empty) always points to a valid value. There are 4 separate stacks: CallStack, ThisStack, ScopeStack and normal Stack.

CallStack is used in a local call mechanism, it stores a return address that points past a call instruction. ThisStack stores “this” pointers for member function calls. ScopeStack contains NULL values that are used as local variables containers (through CScValue properties mechanism). And Stack is the main script stack that is general purpose.

Opcode size is fixed, its size is 4 bytes (DWORD sized). Each instruction consists of an opcode and an optional operand with variable size depending on its type. There are total of 8 types defined for CScValue’s, but only 3 of them are technically used for operands: integer, double and string. Depending on instruction, interpretation of integer value may change, for example, it can also be boolean, an index into symbols table or code offset.

Integer type is a 32-bit integer, double is 64-bit floating-point value and string is a variable sized null-terminated one-byte characters array (cstring).

Instructions🔗

There are 47 instructions defined:

+--------+--------+--------------------------+----------------+
| NUMBER | OPCODE |         MNEMONIC         |     OPERAND    |
+--------+--------+--------------------------+----------------+
|   01   |  0x00  |  "DEF_VAR"               |  Symbol Index  |
|   02   |  0x01  |  "DEF_GLOB_VAR"          |  Symbol Index  |
|   03   |  0x02  |  "RET"                   |      None      |
|   04   |  0x03  |  "RET_EVENT"             |      None      |
|   05   |  0x04  |  "CALL"                  |   Code Offset  |
|   06   |  0x05  |  "CALL_BY_EXP"           |      None      |
|   07   |  0x06  |  "EXTERNAL_CALL"         |  Symbol Index  |
|   08   |  0x07  |  "SCOPE"                 |      None      |
|   09   |  0x08  |  "CORRECT_STACK"         |     Integer    |
|   10   |  0x09  |  "CREATE_OBJECT"         |      None      |
|   11   |  0x0A  |  "POP_EMPTY"             |      None      |
|   12   |  0x0B  |  "PUSH_VAR"              |  Symbol Index  |
|   13   |  0x0C  |  "PUSH_VAR_REF"          |  Symbol Index  |
|   14   |  0x0D  |  "POP_VAR"               |  Symbol Index  |
|   15   |  0x0E  |  "PUSH_VAR_THIS"         |      None      |
|   16   |  0x0F  |  "PUSH_INT"              |     Integer    |
|   17   |  0x10  |  "PUSH_BOOL"             |     Integer    |
|   18   |  0x11  |  "PUSH_FLOAT"            |     Double     |
|   19   |  0x12  |  "PUSH_STRING"           |     String     |
|   20   |  0x13  |  "PUSH_NULL"             |      None      |
|   21   |  0x14  |  "PUSH_THIS_FROM_STACK"  |      None      |
|   22   |  0x15  |  "PUSH_THIS"             |  Symbol Index  |
|   23   |  0x16  |  "POP_THIS"              |      None      |
|   24   |  0x17  |  "PUSH_BY_EXP"           |      None      |
|   25   |  0x18  |  "POP_BY_EXP"            |      None      |
|   26   |  0x19  |  "JMP"                   |   Code Offset  |
|   27   |  0x1A  |  "JMP_FALSE"             |   Code Offset  |
|   28   |  0x1B  |  "ADD"                   |      None      |
|   29   |  0x1C  |  "SUB"                   |      None      |
|   30   |  0x1D  |  "MUL"                   |      None      |
|   31   |  0x1E  |  "DIV"                   |      None      |
|   32   |  0x1F  |  "MODULO"                |      None      |
|   33   |  0x20  |  "NOT"                   |      None      |
|   34   |  0x21  |  "AND"                   |      None      |
|   35   |  0x22  |  "OR"                    |      None      |
|   36   |  0x23  |  "CMP_EQ"                |      None      |
|   37   |  0x24  |  "CMP_NE"                |      None      |
|   38   |  0x25  |  "CMP_L"                 |      None      |
|   39   |  0x26  |  "CMP_G"                 |      None      |
|   40   |  0x27  |  "CMP_LE"                |      None      |
|   41   |  0x28  |  "CMP_GE"                |      None      |
|   42   |  0x29  |  "CMP_STRICT_EQ"         |      None      |
|   43   |  0x2A  |  "CMP_STRICT_NE"         |      None      |
|   44   |  0x2B  |  "DBG_LINE"              |     Integer    |
|   45   |  0x2C  |  "POP_REG1"              |      None      |
|   46   |  0x2D  |  "PUSH_REG1"             |      None      |
|   47   |  0x2E  |  "DEF_CONST_VAR"         |  Symbol Index  |
+--------+--------+--------------------------+----------------+

What exactly each instruction does can be looked in WMEs source code in CScScript::ExecuteInstruction function that is located in ScScript.cpp file.

Global and local scopes🔗

Instructions can be located either in a global scope or local scope. Global scope instructions are not a part of any local function, event or method, unlike local scope ones that do. There may be many local scopes, but only one global. Local scopes are nested inside a global scope and there are no restrictions on where they may be located. Local scope is defined like this:

// <global scope instructions>
  JMP local_scope_end
  SCOPE
  // <local scope instructions>
  RET // or RET_EVENT
local_scope_end:
// <global scope instructions>

Basically, this means that a local function, event or method can be inserted anywhere in a global scope as long as it doesn’t overlap with another local scope. The global code will skip over those parts through an unconditional jump. Each scope must end with either RET or RET_EVENT instruction, otherwise it will attempt to execute code of other scopes or even past code section (which is obviously bad).

Call mechanism🔗

There are 3 types of calls: internal, external and member. For each VM has specific instructions defined — CALL, EXTERNAL_CALL and CALL_BY_EXP respectively. All types pass parameters right-to-left through stack including parameters count. Callee later pops them from stack. Return value is always pushed on stack by callee (event function is the exception here). If it’s meant to have no return value then it pushes NULL and caller pops and discards it.

Internal call is used to call local functions – these are defined within the same script file. A typical call looks like this:

  PUSH_INT 244
  PUSH_BOOL 1
  PUSH_FLOAT 0.244
  PUSH_STRING "Hello, World!"
  PUSH_INT 4 // <-- pushing parameters count

  CALL InternalFunctionName
  POP_EMPTY // <-- discard return value

A callee at InternalFunctionName offset would look something like this:

Note the CORRECT_STACK instruction. It’s used for variable arguments support. More on that later.

  JMP internal_function_end

  SCOPE
  CORRECT_STACK 4
  // <internal function defines>
  // <internal function statements>
  PUSH_NULL // <-- return value
  RET

internal_function_end:

External function call looks exactly the same as internal call, the only difference is call instruction that’s used:

  PUSH_INT 0 // <-- pushing parameters count
  EXTERNAL_CALL ExternalFunctionName
  POP_VAR "variableName" // <-- store return value in a variable

Member call is similar, it only adds “this” pointer push to both ThisStack and Stack and a member function name, also CALL_BY_EXP is used as call instruction:

  PUSH_THIS "objectName" // <-- push "this" to ThisStack

  PUSH_FLOAT 3.1415
  PUSH_INT 1 // <-- pushing parameters count

  PUSH_VAR "objectName" // <-- push "this" to Stack
  PUSH_STRING "functionName"
  CALL_BY_EXP
  POP_EMPTY // <-- discard return value

  POP_THIS // <-- pop "this" from ThisStack

So, if game crashes executing CALL_BY_EXP instruction because of invalid vftable this means that an object pushed by PUSH_VAR is invalid.

Variable parameters count (varargs) is kind of supported, but it’s not what you’d expect it to be. Its implementation depends on CORRECT_STACK instruction. It takes parameters count from stack and compares it to expected number. If there are too many parameters it removes them from stack (starting from the ones pushed earlier) and if there’s too little — NULL values are inserted to match expected parameters count. So it’s possible to pass less parameters to a function than it expects (similar to default arguments in C++ or optional arguments in C# but with value always being NULL), but passing more won’t work since everything above expected parameters count will be removed from stack. This means a classic vararg usage like printf(fmt, ...) is not supported.

Analyzing script disassembly🔗

Now that we know the basics of Wintermute’s virtual machine, we can read and analyze shake.script disassembly. At this point we already know that we need to figure out what object is being passed as “this” pointer into a member call right before 00002B37 address (relative to code section that would be 00002AFC) and where it comes from. We also should keep an eye on anything suspicious that can potentially corrupt this object.

Skipping most of irrelevant parts we end up with this:

See shake.asm for full disassembly.

/*  <....>  */
/* 0000275F */    DBG_LINE 3
/* 00002767 */    DEF_VAR "e"
/* 0000276F */    PUSH_THIS "Scene"
/* 00002777 */    PUSH_STRING "scenes\spr\bricks1.entity"
/* 00002795 */    PUSH_INT 1
/* 0000279D */    PUSH_VAR "Scene"
/* 000027A5 */    PUSH_STRING "LoadEntity"
/* 000027B4 */    CALL_BY_EXP
/* 000027B8 */    POP_THIS
/* 000027BC */    POP_VAR "e"
/*  <....>  */
/* 00002860 */    DBG_LINE 7
/* 00002868 */    DEF_VAR "s"
/* 00002870 */    PUSH_THIS "e"
/* 00002878 */    PUSH_INT 0
/* 00002880 */    PUSH_VAR "e"
/* 00002888 */    PUSH_STRING "GetSpriteObject"
/* 0000289C */    CALL_BY_EXP
/* 000028A0 */    POP_THIS
/* 000028A4 */    POP_VAR "s"
/*  <....>  */
/* 00002986 */    DBG_LINE 19
/* 0000298E */    PUSH_INT 1000
/* 00002996 */    PUSH_INT 60
/* 0000299E */    PUSH_INT 0
/* 000029A6 */    PUSH_INT 2
/* 000029AE */    EXTERNAL_CALL "Random"
/* 000029B6 */    MUL
/* 000029BA */    PUSH_INT 1
/* 000029C2 */    EXTERNAL_CALL "Sleep"
/* 000029CA */    POP_EMPTY

label000029CE: ; 1 reference
/* 000029CE */    DBG_LINE 20
/* 000029D6 */    PUSH_VAR "TRUE"
/* 000029DE */    JMP_FALSE label00002E82
/*  <....>  */
/* 00002A3A */    DBG_LINE 22
/* 00002A42 */    PUSH_THIS "e"
/* 00002A4A */    PUSH_STRING "scenes\spr\bricks"
/* 00002A60 */    PUSH_INT 2
/* 00002A68 */    PUSH_INT 1
/* 00002A70 */    PUSH_INT 2
/* 00002A78 */    EXTERNAL_CALL "Random"
/* 00002A80 */    PUSH_INT 1
/* 00002A88 */    EXTERNAL_CALL "ToString"
/* 00002A90 */    ADD
/* 00002A94 */    PUSH_STRING ".sprite"
/* 00002AA0 */    ADD
/* 00002AA4 */    PUSH_INT 1
/* 00002AAC */    PUSH_VAR "e"
/* 00002AB4 */    PUSH_STRING "SetSprite"
/* 00002AC2 */    CALL_BY_EXP
/* 00002AC6 */    POP_EMPTY
/* 00002ACA */    POP_THIS

/* 00002ACE */    DBG_LINE 23
/* 00002AD6 */    PUSH_THIS "s"
/* 00002ADE */    PUSH_INT 0
/* 00002AE6 */    PUSH_VAR "s"
/* 00002AEE */    PUSH_STRING "Reset"
/* 00002AF8 */    CALL_BY_EXP // <-- this call causes game to crash
/* 00002AFC */    POP_EMPTY
/* 00002B00 */    POP_THIS
/*  <....>  */
/* 00002E2A */    DBG_LINE 42
/* 00002E32 */    PUSH_INT 1000
/* 00002E3A */    PUSH_INT 60
/* 00002E42 */    PUSH_INT 30
/* 00002E4A */    PUSH_INT 2
/* 00002E52 */    EXTERNAL_CALL "Random"
/* 00002E5A */    MUL
/* 00002E5E */    PUSH_INT 1
/* 00002E66 */    EXTERNAL_CALL "Sleep"
/* 00002E6E */    POP_EMPTY

/* 00002E72 */    DBG_LINE 43
/* 00002E7A */    JMP label000029CE

label00002E82: ; 1 reference
/* 00002E82 */    DBG_LINE 1
/* 00002E8A */    JMP label000032EF
/*  <....>  */

It might seem confusing and there’s a lot going on, but in reality it’s not doing much. If we manually decompile it to a high-level representation we will get something like this:

See shake.c for full decompilation.

// <...>
var e = Scene.LoadEntity("scenes\\spr\\bricks1.entity");
// <...>
var s = e.GetSpriteObject();
// <...>
Sleep(Random(0, 60) * 1000);
while (TRUE) {
  // <...>
  e.SetSprite("scenes\\spr\\bricks" + ToString(Random(1, 2)) + ".sprite");
  s.Reset(); // <-- this call causes game to crash
  // <...>
  Sleep(Random(30, 60) * 1000);
}
// <...>

Much better, right? And we can solve one mystery already. Namely, why the time it takes before crash is different each time. That’s because the script is sleeping (paused) for a random amount of time between 0 and 60 seconds (time is passed into Sleep function in milliseconds) before a call to a game crashing function happens.

As for why it crashes, now we can clearly see that object that’s getting corrupted named “s” and it’s returned by GetSpriteObject method on previously loaded entity. We should verify that it returns a valid object and if it does, it means something in-between two calls corrupts it. That SetSprite method looks suspicious and has to be checked too as it might modify our “s” object. Let’s see what it does (it’s located in src/engine_core/wme_ad/AdTalkHolder.cpp:96):

if(strcmp(Name, "SetSprite")==0){
  Stack->CorrectParams(1);
  CScValue* Val = Stack->Pop();
  // <...>
  SAFE_DELETE(m_Sprite);
  if(Val->IsNULL()) {
    // <...>
  } else {
    char* Filename = Val->GetString();
    CBSprite* spr = new CBSprite(Game, this);
    if(!spr || FAILED(spr->LoadFile(Filename))) {
      // <...>
    } else {
      m_Sprite = spr;
      // <...>
    }
  }
  return S_OK;
}

// SAFE_DELETE(m_Sprite) macro expanded:
if(m_Sprite) {
  delete m_Sprite;
  m_Sprite = NULL;
} else 0

First it frees existing m_Sprite object by calling delete operator, then if a file name is provided (as a parameter into this function) a new CBSprite object is created and if it’s successfully loaded it’s stored back into m_Sprite variable which is a pointer to CBSprite object (i.e. CBSprite *).

We’ve now learned that SetSprite function recreates CBSprite object and stores its pointer to m_Sprite variable.

And this how GetSpriteObject is implemented:

else if(strcmp(Name, "GetSpriteObject")==0){
  Stack->CorrectParams(0);

  if(!m_Sprite) Stack->PushNULL();
  else Stack->PushNative(m_Sprite, true);
  return S_OK;
}

It returns whatever is stored in m_Sprite (i.e. a pointer to CBSprite) or NULL if sprite is not initialized. We already know that by the time GetSpriteObject is called m_Sprite variable must’ve been initialized, otherwise we would’ve got a VAL_NULL value from stack, not VAL_NATIVE. Validness of a CBSprite * returned from GetSpriteObject is yet to be confirmed, meanwhile let’s take a look at how this object gets created. Function Scene.LoadEntity is our best place to start (located at src/engine_core/wme_ad/AdScene.cpp:1643):

else if(strcmp(Name, "LoadEntity")==0){
  Stack->CorrectParams(1);
  CAdEntity* ent = new CAdEntity(Game);
  if(ent && SUCCEEDED(ent->LoadFile(Stack->Pop()->GetString()))) {
    AddObject(ent);
    Stack->PushNative(ent, true);
  } else {
    SAFE_DELETE(ent);
    Stack->PushNULL();
  }
  return S_OK;
}

Cutting to the point, m_Sprite is being set to NULL at CAdEntity creation and actually created when loading entity from a file (if it has a sprite defined in it). This means that it should be valid.

At this point we can be almost certain what’s causing the issue. Following shake.script logic it first loads entity from a file and saves a pointer to its sprite object into a variable “s”. Then, right before calling Reset method on it a call to SetSprite is made that recreates sprite and overrides m_Sprite variable. But “s” variable still has a pointer to an old object that’s been deleted, thus variable “s” is invalid now.

In this case WME is trying to call Reset function on a deleted object. When object gets deleted by delete operator a series of destructors is called. One destructor for each object in the inheritance chain (in child-to-parent direction). Each destructor replaces a vftable pointer by the one that corresponds to its object. Once all destructors have been called the memory gets deallocated.

Sure it’s a bit more complicated than this, but that’s enough to get the general idea.

It happens that our CBSprite object’s root is an abstract class CBBase with pure virtual functions in its vftable. That’s where a “pure virtual function call” comes from. As for another crash with no message, it’s just because the memory previously occupied by CBSprite was allocated for something else and got overwritten in-between SetSprite and Reset calls.

Now we have our theory, and it’s time to put it to the test…

Dynamic analysis🔗

Running the game again with debugger attached there are a few things we need to confirm:

CBSprite object returned from GetSpriteObject is valid.
Variable “s” contains valid pointer to CBSprite.
SetSprite method deletes existing CBSprite and allocates new one at different memory address.
Reset method is using now invalid pointer from “s” variable.

Utilizing conditional breakpoints it’s easy to break at the specific function call in the script we want to analyze. There are two breakpoints we have to put initially: at ScCallMethod when it’s about to call GetSpriteObject (at 0043C6A9) and at CanHandleMethod with Reset method name (at 0043C5FA).

Conditional breakpoint for GetSpriteObject function (0043C6A9):

strstr(utf8([ebp + 94]), "shake.script") &&
 streq(utf8([esp + C]), "GetSpriteObject")

First part checks whether currently executing script has a shake.script substring in its file name and second part compares if MethodName argument into ScCallMethod is GetSpriteObject. Since arguments are passed on stack [esp + C] will return fourth argument. Register ebp contains pointer to CScScript object that has m_Filename pointer at offset 0x94.

To learn more about using conditional breakpoints see Expressions and Expression Functions parts of x64dbg documentation.

This condition should be put in Break Condition field inside Edit Breakpoint window:

x64dbg edit breakpoint window — Conditional breakpoint

Conditional breakpoint for Reset function (0043C5FA):

strstr(utf8([ebp + 94]), "shake.script") && streq(utf8([esp]), "Reset")

Now that’s everything is ready it’s time to enter bunker. On a loading screen we should hit our first breakpoint:

x64dbg main window — Conditional breakpoint is hit

Now if we step over the call edx instruction (F8) we can verify whether GetSpriteObject returned a valid object. This returned value is stored on top of a stack, and we need to access and read its data. Here is a series of commands that will do this for us:

// Get script stack array pointer
$stack_ptr = [[ebp + 78] + 24]

// Get m_SP value of script stack
$m_SP = [[ebp + 78] + 34]

// Get native object pointer from a top stack CScValue
$obj = [[$stack_ptr + ($m_SP * 4)] + 2C]

Variable $obj should contain a pointer to CBSprite object if GetSpriteObject returned a valid value. It can be verified by reading a class name from RTTI:

// Get object class name from RTTI
utf8([[[$obj] - 4] + C] + 8)

This returns “.?AVCBSprite@@” — a mangled (decorated) class name which confirms that the object pointer is valid.

Note that MSVC RTTI name mangling differs from non-RTTI. You can refer to StackExchange topic for demangling solutions.

Before doing anything else let’s put a hardware write dword breakpoint at $obj memory address. This will make sure that if anything attempts to rewrite CBSprite’s vftable we’ll break right after an instruction that caused it. It should happen inside a destructor, more specifically CBSprite destructor and then all the parent objects destructors up to CBBase.

Run the game again (F9). After some time between 0 and 60 seconds our hardware breakpoint should be hit inside CBSprite virtual destructor. Going up the call stack reveals that this destructor is called from SetSprite method. While we’re here put a breakpoint at CBSprite’s new operator’s memory allocation call at 004A19B4 to see later the returned address of a new object.

Hit run again. We should immediately break at CBScriptHolder virtual destructor. Checking RTTI with the newly written vftable using “utf8([[[$obj] - 4] + C] + 8)” command shows “.?AVCBScriptHolder@@”, that’s correct. If we continue, the same command will show “.?AVCBScriptable@@” then “.?AVCBNamedObject@@” and finally “.?AVCBBase@@” — the root class of CBSprite.

So far this confirms that our object is being deleted. If we continue program execution we’ll hit a breakpoint at the “new operator” called for CBSprite just before the memory is about to be allocated. After stepping over (F8) we compare $obj and eax values and confirm that they are different. Let’s save this new address:

// Store address of a new CBSprite object
$obj_new = eax

When we run game again a breakpoint at Reset method call is hit (because this is the next function to be executed in the script). Here we can finally confirm whether our theory is correct or not. Let’s update the value stored in the “s” variable: $obj = [esi + 2C] and compare it to $obj_new. They are different! Checking out a class name from the RTTI (with “utf8([[[$obj] - 4] + C] + 8)”) also confirms this is not the object we’d expect to see (i.e. with a class name “.?AVCBSprite@@”) passed into Reset method as “this” pointer. This confirms that variable “s” stores a dangling pointer after SetSprite has been called.

For those of you still wondering why it doesn’t crash on Windows 7 the answer is simple — the pointer returned from a new operator is the same as the old one. Since memory allocation is done through a dynamically linked third-party library the behavior may change. But despite the fact it hasn’t crashed there’s no guarantee it won’t, it’s still a bug, and it should be fixed.

If you hit run again the game will crash.

Solving the issue🔗

Now that we’ve established the invalid sprite object pointer is what’s causing the game to crash, we can think of a solution. So, instead of doing this:

var e = Scene.LoadEntity("scenes\\spr\\bricks1.entity");
var s = e.GetSpriteObject();

while (TRUE) {
  // <...>
  e.SetSprite("scenes\\spr\\bricks" + ToString(Random(1, 2)) + ".sprite");
  s.Reset();
  // <...>
}

We should do this:

var e = Scene.LoadEntity("scenes\\spr\\bricks1.entity");
var s;

while (TRUE) {
  // <...>
  e.SetSprite("scenes\\spr\\bricks" + ToString(Random(1, 2)) + ".sprite");
  s = e.GetSpriteObject();
  s.Reset();
  // <...>
}

This way it is guaranteed that variable “s” will contain a valid pointer after SetSprite method invalidates the previous one (and it’s easy to implement). The var definition is left in the same place because it generates DEF_VAR instruction that initializes variable to NULL which is useless because it’s being rewritten right away. Besides, it’s one instruction less in a loop.

Not like it matters much, but why not? I bet the very same mindset is what probably led to this bug in a first place…

In assembly code it would look like this:

/*  <....>  */
/* 0000275F */    DBG_LINE 3
/* 00002767 */    DEF_VAR "e"
/* 0000276F */    PUSH_THIS "Scene"
/* 00002777 */    PUSH_STRING "scenes\spr\bricks1.entity"
/* 00002795 */    PUSH_INT 1
/* 0000279D */    PUSH_VAR "Scene"
/* 000027A5 */    PUSH_STRING "LoadEntity"
/* 000027B4 */    CALL_BY_EXP
/* 000027B8 */    POP_THIS
/* 000027BC */    POP_VAR "e"
/*  <....>  */
/* 00002860 */    DBG_LINE 7
/* 00002868 */    DEF_VAR "s"
/*  <....>  */
label000029CE: ; 1 reference
/* 000029CE */    DBG_LINE 20
/* 000029D6 */    PUSH_VAR "TRUE"
/* 000029DE */    JMP_FALSE label00002E82
/*  <....>  */
/* 00002A3A */    DBG_LINE 22
/* 00002A42 */    PUSH_THIS "e"
/* 00002A4A */    PUSH_STRING "scenes\spr\bricks"
/* 00002A60 */    PUSH_INT 2
/* 00002A68 */    PUSH_INT 1
/* 00002A70 */    PUSH_INT 2
/* 00002A78 */    EXTERNAL_CALL "Random"
/* 00002A80 */    PUSH_INT 1
/* 00002A88 */    EXTERNAL_CALL "ToString"
/* 00002A90 */    ADD
/* 00002A94 */    PUSH_STRING ".sprite"
/* 00002AA0 */    ADD
/* 00002AA4 */    PUSH_INT 1
/* 00002AAC */    PUSH_VAR "e"
/* 00002AB4 */    PUSH_STRING "SetSprite"
/* 00002AC2 */    CALL_BY_EXP
/* 00002AC6 */    POP_EMPTY
/* 00002ACA */    POP_THIS

                  PUSH_THIS "e"
                  PUSH_INT 0
                  PUSH_VAR "e"
                  PUSH_STRING "GetSpriteObject"
                  CALL_BY_EXP
                  POP_THIS
                  POP_VAR "s"

/* 00002ACE */    DBG_LINE 23
/* 00002AD6 */    PUSH_THIS "s"
/* 00002ADE */    PUSH_INT 0
/* 00002AE6 */    PUSH_VAR "s"
/* 00002AEE */    PUSH_STRING "Reset"
/* 00002AF8 */    CALL_BY_EXP
/* 00002AFC */    POP_EMPTY
/* 00002B00 */    POP_THIS
/*  <....>  */
/* 00002E72 */    DBG_LINE 43
/* 00002E7A */    JMP label000029CE

label00002E82: ; 1 reference
/* 00002E82 */    DBG_LINE 1
/* 00002E8A */    JMP label000032EF
/*  <....>  */

Our solution is ready to be implemented and for that we would need an assembler (had to write that too). The result is single script file shake.script that should be copied to “<game_root>/scenes/bunker” replacing the old one. Quick testing proved it works, the sprite object pointer is always valid and game no longer crashes inside a bunker.

At least not because of this problem. If there’s some other game breaking issue — let the world know, someone will fix it… eventually.

Links🔗

All tools created as a part of this article are written in C# (.NET 6) and released into Public Domain.

Wintermute Engine script disassembler/assembler and dcp package file extractor source code (in a single file): Summerloud.cs

To build copy Summerloud.cs file into an empty folder then run following commands inside that folder (assuming dotnet sdk is installed):

# Step 1: create a new .NET console project
dotnet new console
# Step 2: replace default Program.cs with Summerloud.cs
# On windows: move /Y Summerloud.cs Program.cs
mv -f Summerloud.cs Program.cs
# Step 3: build project
dotnet build

Download fix: shake.script