Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failing to intercept strdup #148

Closed
ramosian-glider opened this issue Aug 31, 2015 · 25 comments
Closed

failing to intercept strdup #148

ramosian-glider opened this issue Aug 31, 2015 · 25 comments

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 148

Repro: http://trac.wxwidgets.org/ticket/15010
Users complain that they see a false report. 
The memory passed to asan's free is coming from strdup, 
which does not get intercepted (and uses libc's malloc which uses sbrk)

I'll try to make a small repro. Meanwhile, any ideas? 

Reported by konstantin.s.serebryany on 2013-02-05 09:34:36

@ramosian-glider
Copy link
Member Author

I tried to compile the wxwidgets code with pure gcc: 

cd wxWidgets/
mkdir build-gcc
cd build-gcc 
../configure
make -j30 
cd samples/minimal/
make
./minimal 

If I add my own implementation of strdup to samples/minimal/minimal.cpp it does not
get called!

diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
index a78e462..d4a2deb 100644
--- a/samples/minimal/minimal.cpp
+++ b/samples/minimal/minimal.cpp
@@ -198,3 +198,13 @@ void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event))
                  wxOK | wxICON_INFORMATION,
                  this);
 }
+extern "C" {
+  char *strdup(const char *x) {
+    fprintf(stderr, "MyStrdup\n");
+    size_t len = strlen(x) + 1;
+    char *res = (char*)malloc(len);
+    memcpy(res, x, len);
+    return res;
+  }
+}


% nm ./minimal | grep strdup 
0000000000409f20 T strdup
% gdb -q ./minimal 
...
(gdb) b strdup
Breakpoint 1 at 0x409f20
(gdb) r
Starting program: /home/kcc/tmp/wxWidgets/build-gcc/samples/minimal/minimal 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/grte/v3/lib64/libthread_db.so.1".

Breakpoint 1, __GI___strdup (s=0x7ffff676eeea <_nl_C_name> "C") at strdup.c:41
41      strdup.c: No such file or directory.
(gdb) bt
#0  __GI___strdup (s=0x7ffff676eeea <_nl_C_name> "C") at strdup.c:41
#1  0x00007ffff71e98d3 in wxLocale::GetSystemEncodingName() () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#2  0x00007ffff71e9aed in wxLocale::GetSystemEncoding() () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#3  0x00007ffff72118a2 in wxCSConv::SetEncoding(wxFontEncoding) () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#4  0x00007ffff721854b in wxCSConv::wxCSConv(wxFontEncoding) () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#5  0x00007ffff7218841 in wxGet_wxConvLocalPtr() () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#6  0x00007ffff7153c02 in _GLOBAL__sub_I_strconv.cpp () from /home/kcc/tmp/wxWidgets/build-gcc/lib/libwx_baseu-2.9.so.5
#7  0x00007ffff7de9306 in call_init (l=<optimized out>, argc=1, argv=0x7fffffffe288,
env=0x7fffffffe298) at dl-init.c:85
#8  0x00007ffff7de93df in call_init (env=<optimized out>, argv=<optimized out>, argc=<optimized
out>, l=<optimized out>) at dl-init.c:52
#9  _dl_init (main_map=0x7ffff7ffe2c8, argc=1, argv=0x7fffffffe288, env=0x7fffffffe298)
at dl-init.c:134
#10 0x00007ffff7ddb6ea in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
#11 0x0000000000000001 in ?? ()
#12 0x00007fffffffe548 in ?? ()
#13 0x0000000000000000 in ?? ()



Reported by konstantin.s.serebryany on 2013-02-05 10:14:34

@ramosian-glider
Copy link
Member Author

FTR, I think I had a similar problem with strdup on Windows.

Reported by timurrrr@google.com on 2013-02-05 10:40:33

@ramosian-glider
Copy link
Member Author

Symbolic linking (wx is a shared library in this setup?)?
Some kind of preprocessor magic?

Reported by eugenis@google.com on 2013-02-05 10:46:17

@ramosian-glider
Copy link
Member Author

Running a pure-gcc build with custom strdup added to samples/minimal/minimal.cpp:
% nm ../../lib/libwx_baseu-2.9.so.5 | grep strdup
                 U strdup@@GLIBC_2.2.5
% nm  minimal | grep strdup
0000000000409f90 T strdup
% 

Disassm for libwx_baseu-2.9.so.5
Dump of assembler code for function _ZN8wxLocale21GetSystemEncodingNameEv:
   0x00007ffff71e9890 <+0>:     mov    %rbx,-0x20(%rsp)
   0x00007ffff71e9895 <+5>:     mov    %rbp,-0x18(%rsp)
   0x00007ffff71e989a <+10>:    xor    %esi,%esi
   0x00007ffff71e989c <+12>:    mov    %r12,-0x10(%rsp)
   0x00007ffff71e98a1 <+17>:    mov    %r13,-0x8(%rsp)
   0x00007ffff71e98a6 <+22>:    sub    $0x68,%rsp
   0x00007ffff71e98aa <+26>:    mov    0x384eff(%rip),%r12        # 0x7ffff756e7b0
   0x00007ffff71e98b1 <+33>:    movq   $0x0,0x8(%rdi)
   0x00007ffff71e98b9 <+41>:    mov    %rdi,%rbx
   0x00007ffff71e98bc <+44>:    lea    0x18(%r12),%rax
   0x00007ffff71e98c1 <+49>:    mov    %rax,(%rdi)
   0x00007ffff71e98c4 <+52>:    xor    %edi,%edi
   0x00007ffff71e98c6 <+54>:    callq  0x7ffff714eda0 <setlocale@plt>
   0x00007ffff71e98cb <+59>:    mov    %rax,%rdi
   0x00007ffff71e98ce <+62>:    callq  0x7ffff7151e90 <strdup@plt>
=> 0x00007ffff71e98d3 <+67>:    mov    %rax,%rbp
   0x00007ffff71e98d6 <+70>:    mov    %rax,%rcx

And this call ends up calling libc's strdup. 

Reported by konstantin.s.serebryany on 2013-02-05 11:42:52

@ramosian-glider
Copy link
Member Author

Ok, mystery solved. 

% head strdup?.cc version-script 
==> strdup1.cc <==
#include <stdio.h>
extern char *z;
int main(int argc, char **argv) {
  printf("z: %p %s\n", z, z);
  return 0;
}

==> strdup2.cc <==
#include <string.h>
char *z = strdup("FFFFF");

==> version-script <==
WXU_2.9 {
        *;
};

% clang++ -fsanitize=address -O3  -fPIC strdup2.cc -shared -o strdup2.so; clang++ -fsanitize=address
strdup2.so ~/tmp/strdup1.cc -Wl,-rpath=.   -Wl,--version-script,./version-script  
&& ./a.out 
z: 0x451f040 FFFFF  <<<<<<<<<<<<<<<< BAD

% clang++ -fsanitize=address -O3  -fPIC strdup2.cc -shared -o strdup2.so; clang++ -fsanitize=address
strdup2.so ~/tmp/strdup1.cc -Wl,-rpath=.  && ./a.out z: 0x60080000dff0 FFFFF  <<<<<<<<<<<<<<<<<<
GOOD


the version script used in the wxWidgets is unfriendly to asan. 
Dunno why (any idea?)
Anyway, I don't consider this an asan bug. 

Reported by konstantin.s.serebryany on 2013-02-05 12:12:33

  • Status changed: WontFix

@ramosian-glider
Copy link
Member Author

Someone needs to read up about versioned symbol lookup, or ask khim.

Reported by eugenis@google.com on 2013-02-05 12:21:41

@ramosian-glider
Copy link
Member Author

Anyway, this is still a bug. Is it possible to at least detect this situation and warn
the user? 

Reported by eugenis@google.com on 2013-02-05 12:24:20

@ramosian-glider
Copy link
Member Author

How? 

Reported by konstantin.s.serebryany on 2013-02-05 12:25:33

@ramosian-glider
Copy link
Member Author

As Victor is too lazy to answer here:
we could use asm(".symver") to pin our interceptors to empty ("") version. This stuff
is stronger than version scripts!

Reported by eugenis@google.com on 2013-02-05 15:09:00

  • Status changed: Started

@ramosian-glider
Copy link
Member Author

Any update on this?

Reported by ettl.martin78 on 2013-02-21 15:19:24

@ramosian-glider
Copy link
Member Author

No. While we may fix this in future, I still suggest to not use the version script with
asan. 

Reported by konstantin.s.serebryany on 2013-02-22 09:13:50

@ramosian-glider
Copy link
Member Author

Can someone please explain what's going on in this bug? I'm hitting this issue in Chrome
when third-party libraries leak the result of strdup:

Indirect leak of 89 byte(s) in 1 object(s) allocated from:
    #0 0x4a3893 in __interceptor_realloc /usr/local/google/work/chromium/src/third_party/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:95
    #1 0x7f1531fda949 in __GI___vasprintf_chk /build/buildd/eglibc-2.15/debug/vasprintf_chk.c:90

Indirect leak of 49 byte(s) in 1 object(s) allocated from:
    #0 0x4a3699 in __interceptor_malloc /usr/local/google/work/chromium/src/third_party/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f1531f58d81 in __GI___strdup /build/buildd/eglibc-2.15/string/strdup.c:43

...

I would like to have interceptors for strdup and vasprintf which would avoid trying
to unwind stack through libc. From this discussion I'm getting the impression that
that is not trivial, but I'm confused as to why.

Reported by earthdok@google.com on 2013-11-28 16:55:34

@ramosian-glider
Copy link
Member Author

FYI: ASan intercepts strdup to make sure we have properly unwinded stack trace,
but not __GI___strdup (I don't know why is it called). IIRC we don't intercept printf
and friends.

Reported by samsonov@google.com on 2013-11-29 08:09:11

@ramosian-glider
Copy link
Member Author

I think we are discussing multiple issues here.
  1. The original bug is about version scripts messing up sanitizer interceptors.
  2. comment #12, based on _chk ending, is due to _FORTIFY_SOURCE. It may be fixed
by intercepting _chk variants, or adding diagnostics, or maybe even undefining _FORTIFY_SOURCE
in the driver.
  3. __GI___strdup is a non-dynamic symbol with the same address as strdup. We don't
need to intercept it, but we may want to tweak the symbolizer to prefer dynamic symbols.

Reported by eugenis@google.com on 2013-11-29 09:49:19

@ramosian-glider
Copy link
Member Author

I don't your comment in (3), this is not related to symbolizer. Apparently, the program
calls __GI__strdup() from libc instead of strdup() from ASan runtime. How can we avoid
this?

Reported by samsonov@google.com on 2013-11-29 09:52:13

@ramosian-glider
Copy link
Member Author

OK. __GI__strdup is still a symbolizer deficiency, but it is not clear why it got called
without going through an interceptor.
My guess is something else in libc called strdup internally. Please re-run the test
with slow unwind in malloc.

Reported by eugenis@google.com on 2013-11-29 10:08:05

@ramosian-glider
Copy link
Member Author

Folks, please stop hijacking a bug. 

Reported by konstantin.s.serebryany on 2013-11-29 10:10:57

@ramosian-glider
Copy link
Member Author

Version scripts can also be used to make certain symbols local. I guess asm(".symver")
workaround won't work in this case.

Ex.: https://android.googlesource.com/platform/art/+/android-l-preview_r2/sigchainlib/version-script.txt

Reported by eugenis@google.com on 2014-12-11 10:15:00

@ramosian-glider
Copy link
Member Author

I don't think people are hijacking the bug - I think attributing this issue version
scripts was inaccurate to begin with. The root cause seems to be ASAN simply not properly
intercepting strdup under any conditions. I'm noticing the same symptoms on my system
and we're not using version scripts. Additionally, the call stack for strdup looks
identical to the examples presented so far - __GI___strdup is called - even without
the use of version scripts.

IMHO, this is a true ASAN bug and it's affecting a lot of code using gcc 4.8.0+ now
because the GCC team has made it available through the -fsanitize=address flag. Of
course, we can always not use that flag, but is that what you guys really want?

Reported by john.calcote on 2015-07-08 19:26:05

@ramosian-glider
Copy link
Member Author

re: #19
Are you using _FORTIFY_SOURCE? Could you provide a small test case, or at least a stack
trace that show where this __GI__strdup is called from?

See #14. This bug is not only about version scripts, but we have not seen any actionable
report of the non-version-script case.

Reported by eugenis@google.com on 2015-07-08 20:03:45

@ramosian-glider
Copy link
Member Author

>> I don't think people are hijacking the bug
That's not the most interesting topic to argue about. 

>> The root cause seems to be ASAN simply not properly intercepting strdup under any
conditions.

False. 

% cat strdup-oob.cc 
#include <string.h>
int main() {
  return strdup("abc")[5];
}
% clang -g -fsanitize=address strdup-oob.cc && ASAN_OPTIONS=strip_path_prefix=$HOME/
./a.out 
=================================================================
==27103==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000eff5 at
pc 0x0000004cde1d bp 0x7ffef4c88c90 sp 0x7ffef4c88c88
READ of size 1 at 0x60200000eff5 thread T0
    #0 0x4cde1c in main tmp/strdup-oob.cc:3:10
    #1 0x7f19fe8e4ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
    #2 0x4178a5 in _start (tmp/a.out+0x4178a5)

0x60200000eff5 is located 1 bytes to the right of 4-byte region [0x60200000eff0,0x60200000eff4)
allocated by thread T0 here:
    #0 0x493543 in strdup llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:590:3
    #1 0x4cdddd in main tmp/strdup-oob.cc:3:10
    #2 0x7f19fe8e4ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287


As you can see, strdup is properly intercepted when using the clang version of AddressSanitizer.



With gcc (4.8.4-2ubuntu1~14.04), it does not happen:

% gcc -g -fsanitize=address strdup-oob.cc && ASAN_OPTIONS=strip_path_prefix=$HOME/
./a.out 2>&1 | ~/llvm/projects/compiler-rt/lib/asan/scripts/asan_symbolize.py 
=================================================================
==27162== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60040000dff5 at
pc 0x400843 bp 0x7ffecc10f940 sp 0x7ffecc10f938
READ of size 1 at 0x60040000dff5 thread T0
LLVMSymbolizer: error reading file: No such file or directory.
addr2line: 'tmp/a.out': No such file
    #0 0x400842 in
    #1 0x7feda6c47ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287:0
    #2 0x400738 in
0x60040000dff5 is located 1 bytes to the right of 4-byte region [0x60040000dff0,0x60040000dff4)
allocated by thread T0 here:
    #0 0x7feda700046a in malloc _asan_rtl_:0
    #1 0x7feda6cae839 in __strdup /build/buildd/eglibc-2.19/string/strdup.c:42:0


But with the more recent gcc (2-3 months old trunk) it works again:
allocated by thread T0 here:
    #0 0x41f65f in __interceptor_strdup ../../../../gcc/libsanitizer/asan/asan_interceptors.cc:514
    #1 0x4a8a2b in main tmp/strdup-oob.cc:3


So, please be more specific about your platform, your compiler version and the exact
problem you experience. 





Reported by konstantin.s.serebryany on 2015-07-08 20:12:53

@ramosian-glider
Copy link
Member Author

Reported by ramosian.glider on 2015-07-30 09:05:31

  • Labels added: ProjectAddressSanitizer

@ramosian-glider
Copy link
Member Author

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:06:55

@kcc
Copy link
Contributor

kcc commented Dec 1, 2015

stale

@kcc kcc closed this as completed Dec 1, 2015
@orbitcowboy
Copy link

I am curious. Is this bug fixed already?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants