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.

Sign up for GitHub

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

Jump to bottom

Double free with AUG_ENABLE_SPAN in free_tree_node #397

Closed
theopolis opened this issue Aug 16, 2016 · 9 comments
Closed

Double free with AUG_ENABLE_SPAN in free_tree_node #397

theopolis opened this issue Aug 16, 2016 · 9 comments

Comments

@theopolis
Copy link

I tried a tiny test:

#include <augeas.h>

int main() {
  augeas *aug = aug_init(nullptr, nullptr, 0);

  if (aug == nullptr) {
    return 1;
  } else {
    aug_close(aug);
  }

  return 0;
}
g++ -I /usr/local/include -std=c++11 -L /usr/local/lib ./main.cpp -l augeas
./a.out

Cool!

Now change:

  augeas *aug = aug_init(nullptr, nullptr, AUG_ENABLE_SPAN);

And it will crash with the double free. Perhaps I'm using the C API incorrectly?

*** Error in `a.out': double free or corruption (fasttop): 0x0000000006353310 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7ff0d1910725]
/lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7ff0d1918f4a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7ff0d191cabc]
a.out[0x16657c1]
a.out(free_tree+0x28)[0x1666d30]
a.out(free_tree+0x1b)[0x1666d23]
a.out(free_tree+0x1b)[0x1666d23]
a.out(free_tree+0x1b)[0x1666d23]
a.out(free_tree+0x1b)[0x1666d23]
a.out(free_tree+0x1b)[0x1666d23]
a.out(aug_close+0x15)[0x1668656]
@theopolis theopolis mentioned this issue Aug 16, 2016
Merged
19 tasks
@lutter
Copy link
Member

lutter commented Aug 16, 2016

I just tried this on my machine (Fedora 23, augeas 1.5.0) and it does not produce an error. Running it under valgrind or with MALLOC_CHECK_=2 in the environment doesn't turn up anything, either. Since you are pulling in stuff from /usr/local, what do you have in there ? Just augeas (and what version) ?

It could be that some config file you have triggers something. Can you try if running AUGEAS_ROOT=/dev/null ./a.out triggers the error, too ? Also, since it's roughly equivalent, what happens when you run augtool --span quit ?

@theopolis
Copy link
Author

Cool! I'll try on a fresh VM with debugging enabled for the few dependencies and see if I can reproduce.

@theopolis
Copy link
Author

I may have been compiling against a newer libz/libxml2 than existed within the system paths. A clean Ubuntu16.04 works fine. Sorry for the noise 💃

@theopolis theopolis reopened this Sep 25, 2016
@theopolis
Copy link
Author

Ok, it seems this is not related to my build, but rather /etc/default/im-config. I reduced the failing content to:

if [ 1 ]; then
# K
else
# I                        
fi

Multiple struct tree*s are assigned the same struct span* leading to a UAF in free_span when trying to unref the span->filename.

@theopolis
Copy link
Author

theopolis commented Sep 25, 2016 •
edited
Loading

With a few added trace statements and ASAN's output:

free_tree: del=0x0x606000fa1420 tree=0x0x606000fa2560 children=0x0x606000fa1360
free_tree: del=0x0x606000fa1360 tree=0x(nil) children=0x0x606000fa0e20
free_tree: del=0x0x606000fa0e20 tree=0x0x606000fa0d60 children=0x(nil)
free_tree_node tree=0x0x606000fa0e20 span=0x0x603001265c20
filename: 0x0x6020013eb1d0 /etc/default/im-config
span: 0x0x603001265c20
free_tree: del=0x0x606000fa0d60 tree=0x(nil) children=0x0x606000fa0be0
free_tree: del=0x0x606000fa0be0 tree=0x(nil) children=0x(nil)
free_tree_node tree=0x0x606000fa0be0 span=0x0x603001267960
filename: 0x0x6020013eb1d0 /etc/default/im-config
span: 0x0x603001267960
free_tree_node tree=0x0x606000fa0d60 span=0x0x603001267cc0
filename: 0x0x6020013eb1d0 /etc/default/im-config
span: 0x0x603001267cc0
free_tree_node tree=0x0x606000fa1360 span=0x0x603001267cc0
=================================================================
==32257==ERROR: AddressSanitizer: heap-use-after-free on address 0x603001267cc0 at pc 0x7fa1c536f180 bp 0x7ffe26cdf6d0 sp 0x7ffe26cdf6c8
READ of size 8 at 0x603001267cc0 thread T0
    #0 0x7fa1c536f17f in free_span /home/teddy/git/augeas/src/info.c:145:5
    #1 0x7fa1c52d6632 in free_tree_node /home/teddy/git/augeas/src/augeas.c:650:9
    #2 0x7fa1c52d6632 in free_tree /home/teddy/git/augeas/src/augeas.c:1141
    #3 0x7fa1c52d65e1 in free_tree /home/teddy/git/augeas/src/augeas.c:1139:16
    #4 0x7fa1c52d65e1 in free_tree /home/teddy/git/augeas/src/augeas.c:1139:16
    #5 0x7fa1c52d65e1 in free_tree /home/teddy/git/augeas/src/augeas.c:1139:16
    #6 0x7fa1c52d65e1 in free_tree /home/teddy/git/augeas/src/augeas.c:1139:16
    #7 0x7fa1c52d65e1 in free_tree /home/teddy/git/augeas/src/augeas.c:1139:16
    #8 0x7fa1c52d7853 in aug_close /home/teddy/git/augeas/src/augeas.c:2096:5
    #9 0x4dc746 in main /home/teddy/git/augeas/src/augtool.c:723:1
    #10 0x7fa1c3b8c82f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291
    #11 0x419418 in _start (/home/teddy/git/augeas/src/.libs/lt-augtool+0x419418)

0x603001267cc0 is located 0 bytes inside of 32-byte region [0x603001267cc0,0x603001267ce0)
freed by thread T0 here:
    #0 0x4afc0b in free /tmp/llvm-20160901-473558-1grod1w/llvm-3.8.1.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38:3
    #1 0x7fa1c52d6632 in free_tree_node /home/teddy/git/augeas/src/augeas.c:650:9
    #2 0x7fa1c52d6632 in free_tree /home/teddy/git/augeas/src/augeas.c:1141

previously allocated by thread T0 here:
    #0 0x4b00b4 in calloc /tmp/llvm-20160901-473558-1grod1w/llvm-3.8.1.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66:3
    #1 0x7fa1c530bbb5 in mem_alloc_n /home/teddy/git/augeas/src/memory.c:66:23
    #2 0x7fa1c536376d in visit_enter /home/teddy/git/augeas/src/get.c:1168:27

SUMMARY: AddressSanitizer: heap-use-after-free /home/teddy/git/augeas/src/info.c:145:5 in free_span
Shadow bytes around the buggy address:
  0x0c0680244f40: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244f50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244f60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0680244f90: fa fa fa fa fa fa fa fa[fd]fd fd fd fa fa fa fa
  0x0c0680244fa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0680244fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==32257==ABORTING

To trigger within augtool --span quit I added an aug_close call:

diff --git a/src/augtool.c b/src/augtool.c
index 1aa58ec..3521fe8 100644
--- a/src/augtool.c
+++ b/src/augtool.c
@@ -720,6 +720,7 @@ int main(int argc, char **argv) {
     if (history_file != NULL)
         write_history(history_file);

+    aug_close(aug);
     return r == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }

@lutter
Copy link
Member

lutter commented Sep 26, 2016

Wow .. that's awesome detective work ! Thanks so much for the great bug report.

I believe the culprit is the aliasing of pointers that happens in visit_enter in get.c

I think I have a fix, but still need to clean it up a little.

lutter added a commit to lutter/augeas that referenced this issue Sep 26, 2016
@lutter
* src/get.c (visit_enter, visit_exit): fix use-after-free when spans …
a8b43c0 
…are used

When spans are enabled, we used to store a a pointer to the same span both
in a frame and in the current state for L_SUBTREE lenses, which meant that
two different tree nodes might have pointers to the same span. That is
counter to the assumptions made about tree nodes, in that they need to own
the spans attached to them, resulting in a use-after-free. It would also
record incorrect span information in one of the tree nodes.  We now
allocate a new span when we store one in the frame - that way we are
guaranteed that there is no aliasing.

With L_MAYBE lenses, visit_enter allocated a new span in state, thus
possibly leaking the one that might be there already. We now store that
span in the frame that we push for L_MAYBE, avoiding that leak.

Finally, we popped frames in visit_exit without doing anything with
them. We now also free any associated span to make sure we do not leak that
information.

Fixes hercules-team#397
@lutter lutter mentioned this issue Sep 26, 2016
Merged
@theopolis
Copy link
Author

Woot! Thanks for jumping on this so quickly @lutter. :)

@theopolis
Copy link
Author

Any interest in adding the aug_close call to src/augtool.c? I originally failed to reproduce because my testing of augtool did not show the UAF or eventual double-free.

@lutter
Copy link
Member

lutter commented Sep 27, 2016

@theopolis I just added a commit to that effect; it's not strictly necessary, but I agree getting error reports over mishandled memory earlier would be good.

@lutter lutter closed this as completed in 323c645 Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@lutter @theopolis