[PATCH] expand: Fix out-of-bounds syntax table access

Ayesha Shafique <[email protected]>
Newsgroups org.kernel.vger.dash
Message-ID <[email protected]>
mbtodest passes a signed byte to chtodest. Bytes with the high bit set
can therefore be negative. The syntax tables are based at SYNBASE so
that signed character values can be used as indices. However, the
is_type path did not apply this offset, causing chtodest and mbtodest
to read before the array:

  $ printf 'v=\377\nw=$v\n' | src/dash
  AddressSanitizer: global-buffer-overflow in chtodest

Apply SYNBASE to is_type, as is already done by the is_special macro.

Fixes: c0c860df08c9 ("expand: Count multi-byte characters for VSLENGTH")
Signed-off-by: Ayesha Shafique <[email protected]>
---

Hi,

I found this while working on a research project, testing against
commit 037bbdf ("Release 0.5.13.5"). Here is the relevant GDB and
AddressSanitizer trace:

  $ printf 'v=\xff\nw=$v\n' > /tmp/s.sh
  $ gdb -q --args ./dash /tmp/s.sh
  Reading symbols from ./dash...
  (gdb) b expand.c:895
  Breakpoint 1 at 0x8c028: file expand.c, line 895.
  (gdb) run
  Starting program: /home/aysha/dash/src/dash /tmp/s.sh

  Breakpoint 1, chtodest (c=-1,
      syntax=0x55555562d680 <is_type> "",
      out=0x5555556bea42 <stackbase+98> "\377") at expand.c:895
  895     {
  (gdb) info args
  c = -1
  syntax = 0x55555562d680 <is_type> ""
  out = 0x5555556bea42 <stackbase+98> "\377"
  (gdb) n
  896             if (syntax[c] == CCTL)
  (gdb) p c
  $1 = -1
  (gdb) n
  =================================================================
  ERROR: AddressSanitizer: global-buffer-overflow on address
  0x55555562d67f
  READ of size 1 at 0x55555562d67f thread T0
      #0 in chtodest /home/aysha/dash/src/expand.c:896
      #1 in mbtodest /home/aysha/dash/src/expand.c:918
      #2 in memtodest /home/aysha/dash/src/expand.c:996
      #3 in strtodest /home/aysha/dash/src/expand.c:1017
      #4 in varvalue /home/aysha/dash/src/expand.c:1140
      #5 in evalvar /home/aysha/dash/src/expand.c:811
      #6 in argstr /home/aysha/dash/src/expand.c:391
      #7 in expandarg /home/aysha/dash/src/expand.c:232
  0x55555562d67f is located 1 bytes before global variable 'is_type'
  defined in 'syntax.c:285:12' (0x55555562d680) of size 257
  SUMMARY: AddressSanitizer: global-buffer-overflow
  /home/aysha/dash/src/expand.c:896 in chtodest

This is my first patch to dash, so apologies for any mistakes I might
have made. I'm happy to fix anything you point out.

Thanks!
Ayesha

 src/expand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/expand.c b/src/expand.c
index 76e67ce..7ebb841 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -980,7 +980,7 @@ static size_t memtodest(const char *p, size_t len, int flags)
 		p += count;
 
 		syntax = flags & (QUOTES_ESC | EXP_MBCHAR) ?
-			 BASESYNTAX : is_type;
+			 BASESYNTAX : is_type + SYNBASE;
 	} else
 		syntax = SQSYNTAX;
 
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.