« May 2009 | Main | July 2009 »
June 25, 2009
Patching a Crash in Kensington MouseWorks
A co-worker had trouble installing the latest version of an application I've been working on. The log file showed that the problem was an access violation when our setup program called MsiInstallProduct. This seemed very unusual, since Windows APIs return error codes instead of crashing. I decided to see where the crash was happening by running the installer under WinDbg.
I set WinDbg to immediately break on exceptions and launched the app. It stopped almost immediately with the following callstack. The crash location is in kwm_dll.dll, which is a hook DLL installed by Kensington MouseWorks.
kmw_dll!CallWndProcFunc+0xa8 USER32!DispatchHookA+0x101 USER32!fnHkINLPCWPSTRUCTA+0x4f USER32!__fnDWORD+0x24 ntdll!KiUserCallbackDispatcher+0x13 USER32!NtUserSetFocus+0xc USER32!CreateDialogIndirectParamAorW+0x33 USER32!CreateDialogParamW+0x49 msi!CBasicUI::CreateProgressDialog+0x35 msi!CBasicUI::CheckDialog+0x47 msi!CBasicUI::SetProgressData+0x58 msi!CBasicUI::Initialize+0x11c msi!MsiUIMessageContext::Initialize+0x230 msi!MsiUIMessageContext::RunInstall+0x22 msi!RunEngine+0xe0 msi!MsiInstallProductW+0xa1 BatchUpd!Application::RunCommandInstall+0x1f5 BatchUpd!Application::Run+0x11e3 BatchUpd!wWinMain+0x102
Disassembling the crash location showed the following (crashing code in bold; ebp does not contain a valid address).
mov eax,dword ptr [ebp-4] mov ecx,dword ptr [eax] push ecx mov edx,dword ptr [ebp-4] mov eax,dword ptr [edx+4] push eax mov ecx,dword ptr [ebp-4] mov edx,dword ptr [ecx+0Ch] push edx call kmw_dll!CallWndProcFunc+0x12f0 (10004210) add esp,0Ch mov eax,dword ptr [ebp+10h] push eax mov ecx,dword ptr [ebp+0Ch] push ecx mov edx,dword ptr [ebp+8] push edx mov eax,dword ptr [kmw_dll!ShowOptsProc+0x6b94 (1000e0a4)] push eax call dword ptr [kmw_dll!ShowOptsProc+0x7c80 (1000f190)] mov esp,ebp pop ebp ret 0Ch
One interesting thing about the code is that it looks like a Debug build (or a Release build with no optimizations): the disassembly is straightforward and seems like it has a one-to-one correspondence with the putative source code. The other thing of note (not shown above) is that the base address of the DLL is set to the default 0x10000000, which is a poor choice for a hook DLL that will be loaded into every process on the system.
ebp should be preserved across the function call, so I looked at the function that was just called (at address 0x10004210). I've added a few explanatory comments based on my understanding of what it's doing.
push ebp ; save caller's value of ebp mov ebp,esp ; standard function prologue sub esp,offset+0x87 (00000088) ; BOOL bLocal0; char szLocal1[132]; cmp dword ptr [ebp+0Ch],0 ; if (param2 == 0) je kmw_dll!CallWndProcFunc+0x130b (1000422b) ; goto label0; mov dword ptr [ebp-88h],0 ; bLocal0 = FALSE; jmp kmw_dll!CallWndProcFunc+0x1315 (10004235) ; goto label1; label0: mov dword ptr [ebp-88h],offset (00000001) ; bLocal0 = TRUE; label1: mov eax,dword ptr [ebp-88h] ; push bLocal0 push eax lea ecx,[ebp-84h] ; push &szLocal1[0] push ecx mov edx,dword ptr [ebp+10h] ; push param3 push edx mov eax,dword ptr [ebp+8] ; push param1 push eax call kmw_dll!ShowOptsProc+0x16e0 (10008bf0) ; fn(param1, param3, szLocal1, bLocal0) add esp,10h ; clean up parameters (C calling convention) mov esp,ebp ; "free" locals pop ebp ; restore caller's value of ebp ret
This function is allocating 0x88 (i.e., 136) bytes for local variable storage: enough for an int (or BOOL) and a 132 byte buffer. If this buffer were overflowed, the stack would be overwritten and ebp would be corrupted upon return. Some internet searching turns up posts that discuss a similar issue, stating that "Kensington MouseWorks ... crashes ... if the executable path is longer than 128 characters"; this seems to match our situation. Indeed, dumping the bytes at the old value of ebp-84h shows the full path of our setup application, which is too long for the buffer.
Since the buffer is stack allocated, it would be trivial to change its size by editing the instructions that create and reference the local variables. At a minimum, the buffer should be capable of storing MAX_PATH characters. Because this function doesn't supply the actual buffer length to the function it calls, we can make it as long as we (reasonably) want. I decided to increase the size for storage of locals in this function to 300 bytes. In version 6.3.2.4 of kmw_dll.dll (which seems like it may be newer than the latest available version, published in February 2006), this can be accomplished by editing the following bytes in the file. These changes simply change the numbers 136, -136, and -132 (which are the three offsets used in the code above) to 300, -300, and -296.
| Offset | New Bytes |
| 0x4215 | 2C 01 |
| 0x4221 | D4 FE |
| 0x422D | D4 FE |
| 0x4237 | D4 FE |
| 0x423E | D8 FE |
The buffer should now be large enough to hold a file name up to MAX_PATH bytes long. With this new DLL installed in the C:\Windows\System32 folder, the setup program is able to launch the MSI and installation completes successfully.
Posted by Bradley Grainger at 7:45 PM | Comments (2) | TrackBack
June 5, 2009
Using If-Modified-Since in HTTP Requests
Conditionally requesting the download of a web page only if it has been modified after a given time seems like it should be as simple as setting the IfModifiedSince property and making the request:
HttpWebRequest request = (HttpWebRequest) WebRequest.Create(@"http://code.logos.com/blog/");
request.IfModifiedSince = new DateTime(2009, 6, 3);
using (HttpWebResponse response = (HttpWebResponse) request.GetResponse())
{
if (response.StatusCode == HttpStatusCode.NotModified)
{
// page wasn't modified; use cached version
}
}
But of course it’s not that simple (as some others have noticed).
The designers of HttpWebRequest decided that some particular HTTP status codes would cause a WebException to be thrown. (As far as I can tell, this list is undocumented, but 304 “Not Modified” is one of them.) This is a vexing exception, because the situation is hardly exceptional. In fact, because it can only happen if IfModifiedSince is explicitly set (or if request.Headers were modified), one could argue that it’s quite expected and intentional. To avoid duplicating logic in the try block (for handling 200 “OK”) and in the catch block (for handling “304” Not Modified), I wrote a utility method that swallows any WebException thrown due to a ProtocolError (e.g., an “invalid” HTTP status code):
public static class HttpWebRequestUtility
{
/// <summary>
/// Gets the <see cref="HttpWebResponse"/> from an Internet resource.
/// </summary>
/// <param name="request">The request.</param>
/// <returns>A <see cref="HttpWebResponse"/> that contains the response from the Internet resource.</returns>
/// <remarks>This method does not throw a <see cref="WebException"/> for "error" HTTP status codes; the caller should
/// check the <see cref="HttpWebResponse.StatusCode"/> property to determine how to handle the response.</remarks>
public static HttpWebResponse GetHttpResponse(this HttpWebRequest request)
{
try
{
return (HttpWebResponse) request.GetResponse();
}
catch (WebException ex)
{
// only handle protocol errors that have valid responses
if (ex.Response == null || ex.Status != WebExceptionStatus.ProtocolError)
throw;
return (HttpWebResponse) ex.Response;
}
}
}
The code to consume this reads very similarly to the first snippet in this post; you just have to remember that normal errors (e.g., 404 “Not Found”) are reported through a valid HttpWebResponse, so its StatusCode property must be checked before acting on the response.
Posted by Bradley Grainger at 11:36 AM | Comments (3) | TrackBack
June 1, 2009
Enumerable.Sum never returns null
I wrote the following code recently, and was surprised when ReSharper warned me that the condition is always true:
IEnumerable<int?> values = // get some values
int? sum = values.Sum();
if (sum.HasValue) { /* this code is always executed */ }
It’s even more surprising when you consider the following difference:
int?[] values = new int?[] { 1, null };
int? sum1 = values.Sum(); // returns 1
int? sum2 = values[0] + values[1]; // returns null
Here, sum1 is 1, but sum2 is null.
Since Sum<int?> never returns null (not even for any empty sequence, or a sequence containing all nulls), it’s odd that its return type is int?, implying that null is a possible return value. Anders explains that this return type is to keep the pattern of T Sum<T>(IEnumerable<T>) for nullable types.
But what if you want Sum to return null if the sequence contains a null? This is easy to simulate using Aggregate, as C# already propagates nulls properly when using the addition operator:
public static class EnumerableUtility
{
public static int? NullableSum(this IEnumerable<int?> values)
{
return values.Aggregate((int?) 0, (sum, value) => sum + value);
}
}
The initial value of 0 is specified to force the sum of an empty list to be zero; you could change it to . A possible optimisation would be to rewrite it with a default(int?) to make an empty list sum to nullforeach loop that returns null as soon as the first null in the sequence is found.
Update: My very smart coworker points out that changing the initial aggregate value to default(int?) makes the function return null for any input. (This is probably a good reason to include a full unit test suite with every blog post…) A custom enumerator (or test of values.Any() first) could be used if returning null as the sum of an empty sequence is desired.
Posted by Bradley Grainger at 4:55 PM | Comments (0) | TrackBack