Bug 81992

Summary: mapi_stub->name points to caller memory leads heap-use-after-free bug
Product: Mesa Reporter: comicfans44 <comicfans44>
Component: GLXAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description comicfans44 2014-08-01 05:46:28 UTC
seems that mapi_stub->name just reference the caller memory instead of copy it.

code path:
glXGetProcessARB -> 
_glapi_get_proc_address->
_glapi_get_stub->
stub_find_dynamic->
stub_add_dynamic->

   stub->name = (const void *) name; -->reference the caller memory

when stub_find_dynamic at 

   for (i = 0; i < count; i++) {
      if (strcmp(name, (const char *) dynamic_stubs[i].name) == 0) {  --->access caller memory
         stub = &dynamic_stubs[i];
         break;
      }
   }

if extension name is allocated at runtime and freed by caller,this bug fired:



    char *p=(char *)malloc(24);

    p[23]='\0';

    strcpy(p,"glFramebufferTextureEXT");
   
    glXGetProcAddressARB((unsigned char*)p);

    free(p);

    glXGetProcAddressARB((unsigned char*)"glFramebufferTextureEXT");

clang reports here:

==2640==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300000c3a2 at pc 0x000000424971 bp 0x7fff00068ab0 sp 0x7fff00068268
READ of size 1 at 0x60300000c3a2 thread T0
    #0 0x424970 in strcmp /run/media/wangxinyu/ubuntu-12-root/home/wangxinyu/llvmgit/clang_fedora/../projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:172
    #1 0x31f4e1225f in stub_find_dynamic (/lib64/libglapi.so.0+0x31f4e1225f)
    #2 0x31f4e120cd in _glapi_get_proc_address (/lib64/libglapi.so.0+0x31f4e120cd)
    #3 0x31f5e1a718 in glXGetProcAddress (/lib64/libGL.so.1+0x31f5e1a718)
    #4 0x4a0906 in initialize() (/home/wangxinyu/glut/a.out+0x4a0906)
    #5 0x4a0f9c in main (/home/wangxinyu/glut/a.out+0x4a0f9c)

0x60300000c3a2 is located 2 bytes inside of 24-byte region [0x60300000c3a0,0x60300000c3b8)
freed by thread T0 here:
    #0 0x48323b in free /run/media/wangxinyu/ubuntu-12-root/home/wangxinyu/llvmgit/clang_fedora/../projects/compiler-rt/lib/asan/asan_malloc_linux.cc:30
    #1 0x4a08f7 in initialize() (/home/wangxinyu/glut/a.out+0x4a08f7)
    #2 0x4a0f9c in main (/home/wangxinyu/glut/a.out+0x4a0f9c)


previously allocated by thread T0 here:
    #0 0x4834bb in __interceptor_malloc /run/media/wangxinyu/ubuntu-12-root/home/wangxinyu/llvmgit/clang_fedora/../projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40
    #1 0x4a078b in initialize() (/home/wangxinyu/glut/a.out+0x4a078b)
    #2 0x4a0f9c in main (/home/wangxinyu/glut/a.out+0x4a0f9c)
    #3 0x31e9621d64 in __libc_start_main (/lib64/libc.so.6+0x31e9621d64)

SUMMARY: AddressSanitizer: heap-use-after-free /run/media/wangxinyu/ubuntu-12-root/home/wangxinyu/llvmgit/clang_fedora/../projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:172 strcmp
Shadow bytes around the buggy address:
  0x0c067fff9820: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9830: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9840: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9850: fa fa fa fa fa fa 00 00 00 06 fa fa 00 00 00 05
  0x0c067fff9860: fa fa 00 00 00 07 fa fa 00 00 00 07 fa fa 00 00
=>0x0c067fff9870: 06 fa fa fa[fd]fd fd fa fa fa fd fd fd fd fa fa
  0x0c067fff9880: fd fd fd fa fa fa 00 00 00 fa fa fa fd fd fd fd
  0x0c067fff9890: fa fa fd fd fd fa fa fa 00 00 00 00 fa fa fd fd
  0x0c067fff98a0: fd fd fa fa fd fd fd fa fa fa fd fd fd fd fa fa
  0x0c067fff98b0: fd fd fd fa fa fa 00 00 00 fa fa fa fd fd fd fd
  0x0c067fff98c0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  ASan internal:           fe
==2640==ABORTING
Comment 1 comicfans44 2015-03-19 05:51:33 UTC
Qt OpenGL use dynamic allocated memory to test GL extensions.
this bug makes every Qt OpenGL app memory corrupted.

I'm not sure a simple strdup is the correct way
(maybe leak instead of memory corrupt). 
with this patch, use-heap-after-free didn't happen anyway.  


diff -Npru mesa-20150314.orig/src/mapi/stub.c mesa-20150314/src/mapi/stub.c
--- mesa-20150314.orig/src/mapi/stub.c  2015-03-14 07:32:12.000000000 +0800
+++ mesa-20150314/src/mapi/stub.c       2015-03-16 10:02:46.860273804 +0800
@@ -110,7 +110,7 @@ stub_add_dynamic(const char *name)
    if (!stub->addr)
       return NULL;
 
-   stub->name = (const void *) name;
+   stub->name = strdup(name);
    /* to be fixed later */
    stub->slot = -1;
Comment 2 Emil Velikov 2015-03-28 23:27:47 UTC
A similar commit landed recently in master and is in mesa 10.5.2. Don't think that we'll be doing any more 10.3 or 10.4 releases, but if we do this commit will be in there.

commit 1110113a7f0b6f9b21dd26dee8e95a021041c71c
Author: Mario Kleiner <mario.kleiner.de@gmail.com>
Date:   Thu Mar 12 23:34:12 2015 +0100

    mapi: Make private copies of name strings provided by client.

    glXGetProcAddress("glFoo") ends up in stub_add_dynamic() to

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.