<rdar://problem/5614257> Crash in timer / hashtable code due to uncaught...
authorsfalken@apple.com <sfalken@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Dec 2007 23:31:46 +0000 (23:31 +0000)
committersfalken@apple.com <sfalken@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Dec 2007 23:31:46 +0000 (23:31 +0000)
        Don't use callback-based timers, since these cause Windows to eat Windows crashes
        in code the timers call.

        Windows appears to be defending against "shatter" attacks partially by setting
        up a structured exception block while dispatching callback-based WM_TIMERs.

        I verified this by adding a divide by zero into some timer callback code.
        In the case where the timer was dispatched via a callback, the divide by zero
        exception was silently handled and ignored, with execution continuing after
        our call to DispatchMessage.  When processed via the WNDPROC, no SEH
        block was established by Windows, and our divide by zero generated a real
        crash (which is what we wanted).

        Windows handling our crashes for us led us to leave the timer data structures
        in an invalid state so the next time a timer was set, we'd crash accessing an
        invalid HashMap of timer data.

        Reviewed by Hyatt.

        * platform/win/SharedTimerWin.cpp:
        (WebCore::TimerWindowWndProc):
        (WebCore::setSharedTimerFireTime):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28500 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/platform/win/SharedTimerWin.cpp

index e4f17ea25dc5f16bb695c02822b1c853acd71249..8f5ea895156fe74e8337f688ca33611b48c3e30c 100644 (file)
@@ -1,3 +1,30 @@
+2007-12-06  Steve Falkenburg  <sfalken@apple.com>
+
+        <rdar://problem/5614257> Crash in timer / hashtable code due to uncaught exception
+        
+        Don't use callback-based timers, since these cause Windows to eat Windows crashes
+        in code the timers call.
+        
+        Windows appears to be defending against "shatter" attacks partially by setting
+        up a structured exception block while dispatching callback-based WM_TIMERs.
+        
+        I verified this by adding a divide by zero into some timer callback code.
+        In the case where the timer was dispatched via a callback, the divide by zero
+        exception was silently handled and ignored, with execution continuing after
+        our call to DispatchMessage.  When processed via the WNDPROC, no SEH
+        block was established by Windows, and our divide by zero generated a real
+        crash (which is what we wanted).
+        
+        Windows handling our crashes for us led us to leave the timer data structures
+        in an invalid state so the next time a timer was set, we'd crash accessing an
+        invalid HashMap of timer data.
+        
+        Reviewed by Hyatt.
+
+        * platform/win/SharedTimerWin.cpp:
+        (WebCore::TimerWindowWndProc):
+        (WebCore::setSharedTimerFireTime):
+
 2007-12-06  Adam Roben  <aroben@apple.com>
 
         Fix <rdar://5108390> Feed title is too low in blue banner
index 455710d8d71b1286b5e158af25729842b5f74bd6..b472764826cbc74b7472f98ecbcd6b8658fb036b 100644 (file)
@@ -41,10 +41,11 @@ static HWND timerWindowHandle = 0;
 static UINT timerFiredMessage = 0;
 const LPCWSTR kTimerWindowClassName = L"TimerWindowClass";
 static bool processingCustomTimerMessage = false;
+const int sharedTimerID = 1000;
 
 LRESULT CALLBACK TimerWindowWndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
 {
-    if (message == timerFiredMessage) {
+    if (message == timerFiredMessage || (message == WM_TIMER && wParam == sharedTimerID)) {
         processingCustomTimerMessage = true;
         sharedTimerFiredFunction();
         processingCustomTimerMessage = false;
@@ -76,11 +77,6 @@ void setSharedTimerFiredFunction(void (*f)())
     sharedTimerFiredFunction = f;
 }
 
-static void CALLBACK timerFired(HWND, UINT, UINT_PTR, DWORD)
-{
-    sharedTimerFiredFunction();
-}
-
 void setSharedTimerFireTime(double fireTime)
 {
     ASSERT(sharedTimerFiredFunction);
@@ -107,13 +103,13 @@ void setSharedTimerFireTime(double fireTime)
     // user input > WM_PAINT/WM_TIMER.)
     // In addition, if the queue contains input events that have been there since the last call to
     // GetQueueStatus, PeekMessage or GetMessage we favor timers.
+    initializeOffScreenTimerWindow();
     if (intervalInMS < USER_TIMER_MINIMUM && !processingCustomTimerMessage && 
         !LOWORD(::GetQueueStatus(QS_ALLINPUT))) {
         // Windows SetTimer does not allow timeouts smaller than 10ms (USER_TIMER_MINIMUM)
-        initializeOffScreenTimerWindow();
         PostMessage(timerWindowHandle, timerFiredMessage, 0, 0);
     } else
-        timerID = SetTimer(0, 0, intervalInMS, timerFired);
+        timerID = SetTimer(timerWindowHandle, sharedTimerID, intervalInMS, 0);
 }
 
 void stopSharedTimer()