Investigating an app compat problem: Part 3: Paydirt
Last time, we learned that the proximate cause of failure was that we were trying to set a bit in a bit array, except that the this pointer was null. That didn’t really bring us any closer to the bug. What we need to do is find out why the calling function tried to invoke the method on a null pointer.
The function that generated the null pointer is kind of long, but let’s see what we can get out of it.
contoso!ContosoInitialize+0x5620: 31426280 push ebp 31426281 mov ebp ,esp 31426283 push 0FFFFFFFFh 31426285 push offset contoso!ContosoInitialize+0xa57c3 (314c6423) 3142628a mov eax, dword ptr fs:[00000000h] 31426290 push eax 31426291 mov dword ptr fs:[0],esp 31426298 sub esp, 24h // 24h bytes of local variables
So far, we have the standard prologue for functions that use exception handling when compiled by the Microsoft C++ compiler. The compiler uses [ebp-4] to keep track of what objects need to be unwound if an exception is raised, so don’t be surprised to see apparent write-only operations to [ebp-4]. These writes are actually clues to us that a stack object with a nontrivial destructor was just constructed or destructed.
3142629b mov dword ptr [ebp-2Ch], ecx // this 3142629e mov eax, dword ptr [ebp-2Ch] 314262a1 mov ecx, dword ptr [eax+400h] // this->m_tlsIndex 314262a7 cmp ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] // some global variable
We learned from our first day that the value at offset 0x400 is a TLS slot index. We compare it against some global variable. What’s up with that global variable?
Let’s search the DLL for all references to that global variable.
0:000> s 31410000 3151e000 9c 69 50 31 3142592c 9c 69 50 31 89 08 8b 55-fc c7 42 04 00 00 00 00 .iP1...U..B..... 31425b6d 9c 69 50 31 74 0c 8b 4d-f0 e8 75 01 00 00 85 c0 .iP1t..M..u..... 31425f9b 9c 69 d2 31 74 11 8b 55-fc 8b 84 95 f8 fe ff ff .iP1t..U........ 31425ffe 9c 69 50 31 89 91 00 04-00 00 8b 45 fc 8b e5 5d .iP1.......E...] 314262a9 9c 69 50 31 75 71 8b 15-ac 69 d2 1f 52 8d 4d e8 .iP1uq...i..R.M. 314262d0 9c 69 50 31 75 3b 6a 00-68 c0 5f c4 1f 8b 55 d4 .iP1u;j.h._...U. 0:000> u 3142592c-2 3142592c contoso!ContosoInitialize+0x4cca: 3142592a mov ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] 0:000> u 31425b6d-2 31425b6d contoso!ContosoInitialize+0x4f0b: 31425b6b cmp eax, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] 0:000> u 31425f9b-2 31425f9b contoso!ContosoInitialize+0x5339: 31425f99 cmp ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] 0:000> u 31425ffe-2 31425ffe contoso!ContosoInitialize+0x539c: 31425ffc mov edx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] 0:000> u 314262a9-2 314262a9 contoso!ContosoInitialize+0x5647: 314262a7 cmp ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] 0:000> u 314262d0-2 314262d0 contoso!ContosoInitialize+0x566e: 314262ce cmp ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)]
It appears that this is a read-only variable. Therefore, its current value is its permanent value. And we saw last time that the permanent value is zero.
And we already found a bug. This code assumes that zero is not a valid TLS index. Actually, the invalid TLS index goes by the name TLS_OUT_OF_INDEXES, which is the value that TlsAlloc uses to say “Sorry, I couldn’t allocate a TLS index for you.” If this app ever calls TlsAlloc and get zero back, it will think that it hasn’t yet assigned a TLS slot.
But that’s not the bug that we’re chasing, because we got a TLS index of 65. But at least we can come up with a nice name for the variable.
DWORD invalidTlsIndex = 0;
Back to the function, already in progress.
314262ad jne contoso!ContosoInitialize+0x56c0 (31426320) // if valid, then skip 314262af mov edx, dword ptr [contoso!ContosoInitialize+0xe5d4c (315069ac)] // get some other thing 314262b5 push edx 314262b6 lea ecx, [ebp-18h] 314262b9 call contoso!ContosoInitialize+0x5170 (31425dd0) // construct something, probably 314262be mov dword ptr [ebp-4], 0 // exception unwinding tracking
I’m guessing that we’re constructing something because it first this pattern: Put into the ecx register the address of some memory never accessed before, then call a function. The object being constructed here doesn’t participate in managing the TLS index: We assume this because it doesn’t take the TLS slot as a parameter. Therefore, we will ignore it for now. (Although I do know what it is, and you might be able to guess too, after we disassemble a little more.)
314262c5 mov eax, dword ptr [ebp-2Ch] // this 314262c8 mov ecx, dword ptr [eax+400h] // this->m_tlsIndex 314262ce cmp ecx, dword ptr [contoso!ContosoInitialize+0xe5d3c (3150699c)] // invalidTlsIndex 314262d4 jne contoso!ContosoInitialize+0x56b1 (31426311) // jump if not equal
This next chunk of code is executed if the TLS index is zero; presumbaly it allocates a TLS slot. Let’s see.
314262d6 push 0 // mystery parameter 314262d8 push offset contoso!ContosoInitialize+0x5360 (31425fc0) // function callback 314262dd mov edx, dword ptr [ebp-2Ch] // this 314262e0 add edx, 400h // &this->m_tlsIndex 314262e6 push edx 314262e7 call contoso!ContosoInitialize+0x6680 (314272e0) // looks promising 314262ec add esp, 0Ch 314262ef test eax, eax 314262f1 je contoso!ContosoInitialize+0x56b1 (31426311) // jump if zero 314262f3 mov dword ptr [ebp-1Ch], 0 // local1c = 0 314262fa mov dword ptr [ebp-4], 0FFFFFFFFh 31426301 lea ecx, [ebp-18h] // destruct that thing on the stack 31426304 call contoso!ContosoInitialize+0x5200 (31425e60) 31426309 mov eax, dword ptr [ebp-1Ch] // return local1c 3142630c jmp contoso!ContosoInitialize+0x5783 (314263e3) 31426311 mov dword ptr [ebp-4], 0FFFFFFFFh 31426318 lea ecx, [ebp-18h] // destruct that thing on the stack 3142631b call contoso!ContosoInitialize+0x5200 (31425e60) ... 314263e3 mov ecx, dword ptr [ebp-0Ch] 314263e6 mov dword ptr fs:[0], ecx 314263ed mov esp, ebp 314263ef pop ebp 314263f0 ret
So far, we have reverse-compiled the code to look like this:
SomeBitArrayClass1* Class2::f_31426280()
{
if (this->m_tlsIndex == invalidTlsIndex)
{
Class3 object3(...);
if (this->m_tlsIndex == invalidTlsIndex)
{
if (f_314272e0(&this->m_tlsIndex, f_31425fc0, 0) != 0)
{
return nullptr;
}
}
}
... more code ...
The Class3 object is probably some sort of synchronization object, since what we have here looks very much like a double-check-locking pattern.
Anyway, that function at 314272e0 probably allocates the TLS slot, seeing as we pass the address of where we want to put the TLS index.
contoso!ContosoInitialize+0x6680: 314272e0 push ebp 314272e1 mov ebp, esp 314272e3 push ecx 314272e4 call dword ptr [contoso!ContosoInitialize+0xa85bc (314c921c)] // TlsAlloc 314272ea mov ecx, dword ptr [ebp+8] // arg1 314272ed mov dword ptr [ecx], eax // save it 314272ef mov edx, dword ptr [ebp+8] 314272f2 cmp dword ptr [edx], 0FFFFFFFFh // was it invalid? 314272f5 je contoso!ContosoInitialize+0x66b3 (31427313) // Y: bail 314272f7 mov eax, dword ptr [ebp+10h] 314272fa push eax 314272fb mov ecx, dword ptr [ebp+0Ch] 314272fe push ecx 314272ff mov edx, dword ptr [ebp+8] 31427302 mov eax, dword ptr [edx] 31427304 push eax 31427305 call contoso!ContosoInitialize+0x53b0 (31426010) // succeeded, keep going 3142730a mov ecx, eax 3142730c call contoso!ContosoInitialize+0x5490 (314260f0) 31427311 jmp contoso!ContosoInitialize+0x6710 (31427370) 31427370 mov esp, ebp 31427372 pop ebp
This function allocates the TLS slot, and if successful, it does something ambiguous. The code seqeuence at 314272f7 could be any of
f_31426010(*tlsIndex, callbackFunction, arg3)->f_314260f0()
f_31426010(*tlsIndex, callbackFunction)->f_314260f0(arg3)
f_31426010(*tlsIndex)->f_314260f0(callbackFunction, arg3)
f_31426010()->f_314260f0(*tlsIndex, callbackFunction, arg3)
If the f_31426010 and f_314260f0 functions were __cdecl, then there would be add esp, N instructions after each call, and that would tell us how many parameters each function consumes. But there isn’t, which means that these functions are __stdcall.
To find out which of the above four cases is the one we have, we need to look at the function epilogue for f_31426010. That will tell us how many bytes of parameters it consumes, and that will tell us which of the parameters belong to f_31426010 and which belong to f_314260f0.
contoso!ContosoInitialize+0x53b0: 31426010 push ebp 31426011 mov ebp,esp ... 314260ea mov esp,ebp 314260ec pop ebp 314260ed ret
Okay, the function ends with a plain ret, which combined with the lack of add esp, N in the calling code means that it consumes zero parameters from the stack.
Therefore, we are in this case:
f_31426010()->f_314260f0(*tlsIndex, callbackFunction, arg3)
The parameters (including the TLS slot index that we are tracking very closely) all go to the f_314260f0 function.
contoso!ContosoInitialize+0x5490: 314260f0 push ebp 314260f1 mov ebp, esp 314260f3 push 0FFFFFFFFh 314260f5 push offset contoso!ContosoInitialize+0xa5778 (314c63d8) 314260fa mov eax, dword ptr fs:[00000000h] 31426100 push eax 31426101 mov dword ptr fs:[0], esp 31426108 sub esp, 28h 3142610b mov dword ptr [ebp-34h], ecx // save "this" 3142610e mov eax, dword ptr [contoso!ContosoInitialize+0xe5d4c (315069ac)] 31426113 push eax 31426114 lea ecx, [ebp-18h] 31426117 call contoso!ContosoInitialize+0x5170 (31425dd0) // construct Class3 3142611c mov dword ptr [ebp-4], 0 31426123 mov ecx, dword ptr [ebp+8] // tlsIndex 31426126 mov dword ptr [ebp-10h], ecx // local10 = tlsIndex 31426129 cmp dword ptr [ebp-10h], 40h // tlsIndex compared with 64 3142612d jae contoso!ContosoInitialize+0x551f (3142617f) // Jump if tlsIndex >= 64 3142617f mov dword ptr [ebp-30h], 0FFFFFFFFh // local30 = -1 31426186 mov dword ptr [ebp-4], 0FFFFFFFFh 3142618d lea ecx, [ebp-18h] 31426190 call contoso!ContosoInitialize+0x5200 (31425e60) // destruct Class3 31426195 mov eax, dword ptr [ebp-30h] // return local30 31426198 mov ecx, dword ptr [ebp-0Ch] 3142619b mov dword ptr fs:[0], ecx 314261a2 mov esp, ebp 314261a4 pop ebp 314261a5 ret 0Ch
Aha! We see that the code checks the numeric value of the TLS index. The only meaningful value to compare the index against is TLS_OUT_OF_INDEXES. Once you verify that you have a valid TLS index, the actual numeric value is opaque. Changing behavior based on the numeric value of the slot index is highly suspect.
The partially-reverse-compiled function looks like this:
int f_314260f0(DWORD tlsIndex, callback, x)
{
Class3 object3(...);
if (tlsIndex < 64)
{
...
}
else
{
return -1;
}
}
Holy cow, this function simply rejects TLS slot indices that are greater than or equal to 64! Returning -1 causes the calling function f_31426280 to return nullptr, which leads to our null pointer crash.
Now we understand why the program is crashing. It mishandles TLS slot indices that are 64 or higher. It tries to reject them, but notice that when function f_314260f0 returns -1, the calling function does not free the TLS slot or reset this->m_tlsIndex back to invalidTlsIndex. Instead, it leaves the TLS slots index allocated, and the next time the code wants to use the object, it sees that the TLS slot index is valid and tries to use the value stored in that slot.
Except we never stored anything there.
What’s so special about the number 64? We’ll dig into that next time.
Light
Dark
0 comments