USER32: accelerator table fixes

Mike McCormack mike at codeweavers.com
Sat Jul 10 10:51:17 CDT 2004


1. CopyAcceleratorTable can cause a buffer overflow because it uses an
    incorrect comparison between the number of accelerator entries
    available and the number of accelerator entries in the output buffer.

2. My tests (a later patch) show that CopyAcceleratorTable always strips
    the high bit of the fVirt member of the accel struct.

3. Calling DestroyAcceleratorTable with a NULL accelerator should return
    FALSE but doesn't.

-------------- next part --------------
Index: dlls/user/resource.c
===================================================================
RCS file: /home/wine/wine/dlls/user/resource.c,v
retrieving revision 1.22
diff -u -r1.22 resource.c
--- dlls/user/resource.c	7 Apr 2004 19:41:21 -0000	1.22
+++ dlls/user/resource.c	10 Jul 2004 15:43:57 -0000
@@ -160,7 +160,7 @@
     return 0;
   }
   xsize = GlobalSize16(HACCEL_16(src))/sizeof(ACCEL16);
-  if (xsize>entries) entries=xsize;
+  if (xsize<entries) entries=xsize;
 
   i=0;
   while(!done) {
@@ -171,15 +171,13 @@
     /* Copy data to the destination structure array (if dst == NULL,
        we're just supposed to count the number of entries). */
     if(dst) {
-      dst[i].fVirt = accel[i].fVirt;
+      dst[i].fVirt = accel[i].fVirt&0x7f;
       dst[i].key = accel[i].key;
       dst[i].cmd = accel[i].cmd;
 
       /* Check if we've reached the end of the application supplied
          accelerator table. */
       if(i+1 == entries) {
-	/* Turn off the high order bit, just in case. */
-	dst[i].fVirt &= 0x7f;
 	done = TRUE;
       }
     }
@@ -308,6 +306,8 @@
  */
 BOOL WINAPI DestroyAcceleratorTable( HACCEL handle )
 {
+    if( !handle )
+        return FALSE;
     return !GlobalFree16(HACCEL_16(handle));
 }
 


More information about the wine-patches mailing list