On Thu, Jun 15, 2017 at 09:41:08AM -0500, Eduardo Bustamante wrote:
> Found by fuzzing `read -e' with AFL. The stacktrace reported by Address
> Sanitizer is followed by the base64 encoded crashing input.
>
>
> ==1098==ERROR: AddressSanitizer: global-buffer-overflow on address
> 0x55e61a6b4c5c at pc 0x55e61a3426ca bp 0x7fff1820a300 sp 0x7fff1820a2f8
> READ of size 4 at 0x55e61a6b4c5c thread T0
> #0 0x55e61a3426c9 in bash_dequote_filename
> (/home/dualbus/src/gnu/bash-build/bash+0x17a6c9)
Easy fix. `p' is a signed char pointer, therefore when `*p = 255', it tries to
dereference `sh_syntaxtab[-1]'.
dualbus@debian:~/src/gnu/bash$ git difftool -y -x 'diff -c' -- bashline.c
*** /tmp/mVc4sH_bashline.c 2017-06-16 09:52:56.471508904 -0500
--- bashline.c 2017-06-16 09:48:55.706503276 -0500
***************
*** 3886,3892 ****
*r++ = *p;
/* Backslashes are preserved within double quotes unless the
character is one that is defined to be escaped */
! else if (quoted == '"' && ((sh_syntaxtab[p[1]] & CBSDQUOTE) == 0))
*r++ = *p;
*r++ = *++p;
--- 3886,3892 ----
*r++ = *p;
/* Backslashes are preserved within double quotes unless the
character is one that is defined to be escaped */
! else if (quoted == '"' && ((sh_syntaxtab[(unsigned char)p[1]] &
CBSDQUOTE) == 0))
*r++ = *p;
*r++ = *++p;
Maybe it's a good idea to change these too. In locale.c there shouldn't be a
problem, because the loop is constrained to `x < sh_syntabsiz', but perhaps
just to silence compiler warnings :-)?
dualbus@debian:~/src/gnu/bash$ git difftool -y -x 'diff -c' -- locale.c parse.y
*** /tmp/OyLWya_locale.c 2017-06-16 09:55:15.854368199 -0500
--- locale.c 2017-06-16 09:50:52.816950476 -0500
***************
*** 552,565 ****
for (x = 0; x < sh_syntabsiz; x++)
{
if (isblank ((unsigned char)x))
! sh_syntaxtab[x] |= CSHBRK|CBLANK;
else if (member (x, shell_break_chars))
{
! sh_syntaxtab[x] |= CSHBRK;
! sh_syntaxtab[x] &= ~CBLANK;
}
else
! sh_syntaxtab[x] &= ~(CSHBRK|CBLANK);
}
}
--- 552,565 ----
for (x = 0; x < sh_syntabsiz; x++)
{
if (isblank ((unsigned char)x))
! sh_syntaxtab[(unsigned char)x] |= CSHBRK|CBLANK;
else if (member (x, shell_break_chars))
{
! sh_syntaxtab[(unsigned char)x] |= CSHBRK;
! sh_syntaxtab[(unsigned char)x] &= ~CBLANK;
}
else
! sh_syntaxtab[(unsigned char)x] &= ~(CSHBRK|CBLANK);
}
}
*** /tmp/qKFxWa_parse.y 2017-06-16 09:55:15.862368362 -0500
--- parse.y 2017-06-16 09:52:22.522808775 -0500
***************
*** 4842,4848 ****
/* If the next character is to be quoted, note it now. */
if (cd == 0 || cd == '`' ||
! (cd == '"' && peek_char >= 0 && (sh_syntaxtab[peek_char] &
CBSDQUOTE)))
pass_next_character++;
quoted = 1;
--- 4842,4848 ----
/* If the next character is to be quoted, note it now. */
if (cd == 0 || cd == '`' ||
! (cd == '"' && peek_char >= 0 && (sh_syntaxtab[(unsigned
char)peek_char] & CBSDQUOTE)))
pass_next_character++;
quoted = 1;
--
Eduardo Bustamante
https://dualbus.me/
