- 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. - My tests show that CopyAcceleratorTable always strips the high bit of the fVirt member of the accel struct. - Calling DestroyAcceleratorTable with a NULL accelerator should return FALSE.

- CopyAcceleratorTable can cause a buffer overflow because it uses an
9243c96e · Mike McCormack · Alexandre Julliard · 74cebde2 · 9243c96e
Commit 9243c96e authored Jul 12, 2004 by Mike McCormack Committed by Alexandre Julliard Jul 12, 2004
Hide whitespace changes
Inline Side-by-side

Showing with 4 additions and 4 deletions

resource.c dlls/user/resource.c +4 -4

No files found.
--- a/dlls/user/resource.c
+++ b/dlls/user/resource.c
@@ -160,7 +160,7 @@ INT WINAPI CopyAcceleratorTableW(HACCEL src, LPACCEL dst,
    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 @@ INT WINAPI CopyAcceleratorTableW(HACCEL src, LPACCEL dst,
    /* 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 @@ HACCEL WINAPI CreateAcceleratorTableW(LPACCEL lpaccel, INT cEntries)
 */
 BOOL WINAPI DestroyAcceleratorTable( HACCEL handle )
 {
+    if( !handle )
+        return FALSE;
    return !GlobalFree16(HACCEL_16(handle));
 }