Re: AddressSanitizer: heap-buffer-overflow lib/readline/bind.c:437 in rl_translate_keyseq

[Eduardo Bustamante]

On Tue, May 2, 2017 at 9:04 AM, Chet Ramey <chet.ramey@case.edu> wrote:
[...]

dualbus@debian:~/src/gnu/bash$ valgrind --leak-check=full ./bash
==30183== Memcheck, a memory error detector
==30183== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30183== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==30183== Command: ./bash
==30183==
bash-4.4$ echo $BASH_VERSION
4.4.12(2)-maint
bash-4.4$ bind '\:""'
bash-4.4$ exit
==30183==
==30183== HEAP SUMMARY:
==30183==     in use at exit: 435,448 bytes in 6,815 blocks
==30183==   total heap usage: 8,384 allocs, 1,569 frees, 867,035 bytes allocated
==30183==
==30183== 1 bytes in 1 blocks are definitely lost in loss record 2 of 387
==30183==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==30183==    by 0x1AA6C9: xmalloc (xmalloc.c:112)
==30183==    by 0x1EDB6A: rl_macro_bind (bind.c:308)
==30183==    by 0x1F0010: rl_parse_and_bind (bind.c:1461)
==30183==    by 0x1AB06A: bind_builtin (bind.def:282)
==30183==    by 0x156033: execute_builtin (execute_cmd.c:4605)
==30183==    by 0x156F0F: execute_builtin_or_function (execute_cmd.c:5103)
==30183==    by 0x155939: execute_simple_command (execute_cmd.c:4391)
==30183==    by 0x14F2EF: execute_command_internal (execute_cmd.c:811)
==30183==    by 0x14E84B: execute_command (execute_cmd.c:393)
==30183==    by 0x1389C9: reader_loop (eval.c:172)
==30183==    by 0x136616: main (shell.c:794)
==30183==
==30183== LEAK SUMMARY:
==30183==    definitely lost: 1 bytes in 1 blocks
==30183==    indirectly lost: 0 bytes in 0 blocks
==30183==      possibly lost: 0 bytes in 0 blocks
==30183==    still reachable: 435,447 bytes in 6,814 blocks
==30183==         suppressed: 0 bytes in 0 blocks
==30183== Reachable blocks (those to which a pointer was found) are not shown.
==30183== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==30183==
==30183== For counts of detected and suppressed errors, rerun with: -v
==30183== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

FWIW, I built above bash with:

$ CFLAGS='-ggdb -O0' ./configure --without-bash-malloc

And:

dualbus@debian:~/src/gnu/bash$ ./bash
bash-4.4$ echo $BASH_VERSION
4.4.12(1)-maint
bash-4.4$ bind '\:""'
bash-4.4$ exit

=================================================================
==15814==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7fe669e16d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
    #1 0x5557622e653f in xmalloc /home/dualbus/src/gnu/bash/xmalloc.c:112
    #2 0x5557623714ff in rl_macro_bind
/home/dualbus/src/gnu/bash/lib/readline/bind.c:308
    #3 0x555762377445 in rl_parse_and_bind
/home/dualbus/src/gnu/bash/lib/readline/bind.c:1461
    #4 0x5557622e759d in bind_builtin bind.def:282
    #5 0x55576222093a in execute_builtin
/home/dualbus/src/gnu/bash/execute_cmd.c:4605
    #6 0x555762222550 in execute_builtin_or_function
/home/dualbus/src/gnu/bash/execute_cmd.c:5103
    #7 0x55576221fdd0 in execute_simple_command
/home/dualbus/src/gnu/bash/execute_cmd.c:4391
    #8 0x55576220dccf in execute_command_internal
/home/dualbus/src/gnu/bash/execute_cmd.c:811
    #9 0x55576220c402 in execute_command
/home/dualbus/src/gnu/bash/execute_cmd.c:393
    #10 0x5557621dc606 in reader_loop /home/dualbus/src/gnu/bash/eval.c:172
    #11 0x5557621d79ed in main /home/dualbus/src/gnu/bash/shell.c:794
    #12 0x7fe6695a92b0 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: 1 byte(s) leaked in 1 allocation(s).

With:

$ CFLAGS='-O0 -ggdb -fsanitize=address -fno-omit-frame-pointer'
LDFLAGS='-lasan' ./configure --without-bash-malloc

dualbus@debian:~/src/gnu/bash$ git rev-parse HEAD
2a39157723ffb7dfc597dfa46b5b6fbd93cc9ea2

I think this is the fix:

dualbus@debian:~/src/gnu/bash$ git diff
diff --git a/lib/readline/bind.c b/lib/readline/bind.c
index 117f3427..e02750d6 100644
--- a/lib/readline/bind.c
+++ b/lib/readline/bind.c
@@ -440,8 +440,10 @@ rl_translate_keyseq (const char *seq, char
*array, int *len)
        {
          c = seq[++i];

-         if (c == 0)
+         if (c == 0) {
+           l++;
            break;
+         }

          /* Handle \C- and \M- prefixes. */
          if ((c == 'C' || c == 'M') && seq[i + 1] == '-')