Address sanitizer reports error

[mlundh]

Hi, I am using editline in a project where I am also enabling address sanitizer. I get a buffer overrun error in completion.c:203. I am not quite sure what the correct way of fixing the issue is, but it seems as the memcpy is trying to read beyond the allocated memory in the av[0] array. Reducing the size of the copy (+1 instead of +2 on line 200) resolves that particular issue, but I can not guarantee that it will not break anything else.

[troglobit]

Can you show me how to reproduce this?

[troglobit]

Because running ./configure CFLAGS="-g -fsanitize=address" followed by a clean make gives me nothing.

[troglobit]

... wait a second!

jocke@luthien:~/src/editline [master]$ find . -name completion.c
jocke@luthien:~/src/editline [master]$ find . -name complete.c
./src/complete.c

I think you've reported a problem in the wrong project. We don't have the file you mention.

Comments?

[mlundh]

I got the name of the file wrong, is in fact complete.c, just as you say. I apologize for that.

I looked a bit closer at the issue, and I can reproduce this by configuring with: ./configure CFLAGS="-g -fno-omit-frame-pointer -fsanitize=address" LDFLAGS="-fno-omit-frame-pointer -fsanitize=address"

after building the library, I also rebuild the examples. Running the fileman example and typing /de and then tab will cause the issue for me.

I am using gcc 9.3.0

[troglobit]

OK, many people confuse this project with libedit, so ... :)

When I run fileman with the above, like you also on gcc 9.3.0. I get a double free rather than a buffer overrun:

FileMan: /defree(): double free detected in tcache 2
Aborted (core dumped)

When I run gdb on it, I end up in comple.c but on line 374.

#3  0x00007ffff7e5047c in malloc_printerr (str=str@entry=0x7ffff7f745d0 "free(): double free detected in tcache 2") at malloc.c:5347
No locals.
#4  0x00007ffff7e520ed in _int_free (av=0x7ffff7fa3b80 <main_arena>, p=0x55555556b800, have_lock=0) at malloc.c:4201
        tmp = <optimized out>
        e = 0x55555556b810
        tc_idx = 0
        size = 32
        fb = <optimized out>
        nextchunk = <optimized out>
        nextsize = <optimized out>
        nextinuse = <optimized out>
        prevsize = <optimized out>
        bck = <optimized out>
        fwd = <optimized out>
        __PRETTY_FUNCTION__ = "_int_free"
#5  0x000055555555af6c in complete (match=0x7fffffffda64, token=0x555555562530 "/de") at complete.c:374
        i = <optimized out>
        len = <optimized out>
        word = 0x555555562550 "UUU"
        words = 0x555555562570
        end = <optimized out>
        start = 0
        len = <optimized out>
        word = <optimized out>
        words = <optimized out>
        start = <optimized out>
        end = <optimized out>
        i = <optimized out>

Not sure atm. why, but the fileman example was "recently" ported from GNU readline, and the completion framework in editline that b0rks was added specifically for GNU readline API compatibility. The cli example uses the traditional API.

[mlundh]

Strange, I can get the issue quite reliably whenever I try complete a filename that is not in the current directory, and not just using fileman, but also testit. I do not think that cli has the ability to complete a filename.

Another strange thing is that I have to reset the terminal after the issue, but that might be because no clean up is performed?

The error I am seeing is shown below. I removed my local path and just left the editline part :)

testit> ../../=================================================================
==77094==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000099 at pc 0x7f8776234480 bp 0x7ffd8c66b460 sp 0x7ffd8c66ac08
READ of size 10 at 0x602000000099 thread T0
    #0 0x7f877623447f  (/lib/x86_64-linux-gnu/libasan.so.5+0x9b47f)
    #1 0x563d95bd4737 in el_filename_complete editline/src/complete.c:203
    #2 0x563d95bd58c3 in rl_complete editline/src/complete.c:397
    #3 0x563d95bd2637 in c_complete editline/src/editline.c:1704
    #4 0x563d95bd0d70 in emacs editline/src/editline.c:1111
    #5 0x563d95bd0f32 in editinput editline/src/editline.c:1194
    #6 0x563d95bd1d77 in readline editline/src/editline.c:1573
    #7 0x563d95bcd943 in main editline/examples/testit.c:54
    #8 0x7f8775fce0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #9 0x563d95bcd79d in _start (editline/examples/testit+0x479d)

0x602000000099 is located 0 bytes to the right of 9-byte region [0x602000000090,0x602000000099)
allocated by thread T0 here:
    #0 0x7f877622f3dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
    #1 0x563d95bd3eeb in FindMatches editline/src/complete.c:91
    #2 0x563d95bd4557 in el_filename_complete editline/src/complete.c:188
    #3 0x563d95bd58c3 in rl_complete editline/src/complete.c:397
    #4 0x563d95bd2637 in c_complete editline/src/editline.c:1704
    #5 0x563d95bd0d70 in emacs editline/src/editline.c:1111
    #6 0x563d95bd0f32 in editinput editline/src/editline.c:1194
    #7 0x563d95bd1d77 in readline editline/src/editline.c:1573
    #8 0x563d95bcd943 in main editline/examples/testit.c:54
    #9 0x7f8775fce0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/lib/x86_64-linux-gnu/libasan.so.5+0x9b47f) 
Shadow bytes around the buggy address:
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff8000: fa fa 01 fa fa fa 07 fa fa fa 07 fa fa fa 01 fa
=>0x0c047fff8010: fa fa 00[01]fa fa 00 03 fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8060: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc
==77094==ABORTING

[mlundh]

Hi again, I kept looking at the code, and I am fairly certain that line 200 in complete.c j = strlen(av[0]) - len + 2; should really be j = strlen(av[0]) - len + 1; since I think that the only reason for +1 is to accommodate the null character. If we keep the +2 then we will always read beyond the null character of the av[0] string on line 203: memcpy(p, av[0] + len, j); since j will always be 1 larger than av[0] + len including the null character of the av[0] string.

Do you think this could be right, or am I just overthinking it? :)

[troglobit]

No that sounds entirely reasonable. Care to submit a pull request?

[troglobit]

Another strange thing is that I have to reset the terminal after the issue, but that might be because no clean up is performed?

Yup, that sounds likely.

[mlundh]

No that sounds entirely reasonable. Care to submit a pull request?

Will do! :)

I notice another issue in the fileman example after fixing this, a use after free. I might look into that after this, but will create a different pull request if I find any proper solution for that.

[troglobit]

Awesome, thank you for the hard work and your contributions this far! <3

[mlundh]

No problem, happy to contribute even if it is in such small way :)