Heap-based buffer overflow in check_literal()

[else]

On Sun, Oct 18, 2015, at 18:02, Jakub Wilk wrote:

Package: jq
Version: 1.5+dfsg-1
Usertags: afl

There's heap-based buffer overflow in check_literal():

$ valgrind jq . overflow.json
==2609== Memcheck, a memory error detector
==2609== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==2609== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright
info
==2609== Command: jq . overflow.json
==2609==
==2609== Invalid write of size 1
==2609== at 0x12D76B: check_literal (jv_parse.c:488)
==2609== by 0x12E930: jv_parser_next (jv_parse.c:782)
==2609== by 0x14072C: jq_util_input_next_input (util.c:444)
==2609== by 0x10D3C9: main (main.c:527)
==2609== Address 0x4b7c5e8 is 0 bytes after a block of size 256 alloc'd
==2609== at 0x482A1DC: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2609== by 0x482C3D0: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2609== by 0x13B375: jv_mem_realloc (jv_alloc.c:162)
==2609== by 0x12D238: tokenadd (jv_parse.c:388)
==2609== by 0x12E058: scan (jv_parse.c:626)
==2609== by 0x12E641: jv_parser_next (jv_parse.c:743)
==2609== by 0x14072C: jq_util_input_next_input (util.c:444)
==2609== by 0x10D3C9: main (main.c:527)
...

(Note that I rebuilt the package with noopt to make the backtrace more
useful.)

This bug was found using American fuzzy lop:
http://lcamtuf.coredump.cx/afl/

see https://bugs.debian.org/802231

[dtolnay]

Interesting, that exact line is commented // FIXME: invalid, and the previous line is // FIXME: better parser. I am not familiar with this part of the code but it looks like it is adding a null character onto the end of the current token in order to call dtoa, without making room for it in the buffer. Definitely needs to be fixed.

From the Debian ticket, the AFL input was:

[0,true,false,0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

[dtolnay]

Interesting, that exact line is commented // FIXME: invalid, and the previous line is // FIXME: better parser. I am not familiar with this part of the code but it looks like it is adding a null character onto the end of the current token in order to call dtoa, without making room for it in the buffer. Definitely needs to be fixed.

From the Debian ticket, the AFL input was:

[0,true,false,0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

[nicowilliams]

Good catch! That FIXME comment and this bug go back a long time. The bug is an off by one around resizing the token buffer.

I'm currently testing this fix:

diff --git a/src/jv_parse.c b/src/jv_parse.c
index 3102ed4..b9b6348 100644
--- a/src/jv_parse.c
+++ b/src/jv_parse.c
@@ -383,7 +383,7 @@ static pfunc stream_token(struct jv_parser* p, char ch) {

 static void tokenadd(struct jv_parser* p, char c) {
   assert(p->tokenpos <= p->tokenlen);
-  if (p->tokenpos == p->tokenlen) {
+  if (p->tokenpos >= (p->tokenlen - 1)) {
     p->tokenlen = p->tokenlen*2 + 256;
     p->tokenbuf = jv_mem_realloc(p->tokenbuf, p->tokenlen);
   }

[nicowilliams]

Good catch! That FIXME comment and this bug go back a long time. The bug is an off by one around resizing the token buffer.

I'm currently testing this fix:

diff --git a/src/jv_parse.c b/src/jv_parse.c
index 3102ed4..b9b6348 100644
--- a/src/jv_parse.c
+++ b/src/jv_parse.c
@@ -383,7 +383,7 @@ static pfunc stream_token(struct jv_parser* p, char ch) {

 static void tokenadd(struct jv_parser* p, char c) {
   assert(p->tokenpos <= p->tokenlen);
-  if (p->tokenpos == p->tokenlen) {
+  if (p->tokenpos >= (p->tokenlen - 1)) {
     p->tokenlen = p->tokenlen*2 + 256;
     p->tokenbuf = jv_mem_realloc(p->tokenbuf, p->tokenlen);
   }

[nicowilliams]

Fixed with 8eb1367.

[nicowilliams]

Fixed with 8eb1367.

[dolmen]

This issue is referenced as CVE-2015-8863.

[dolmen]

This issue is referenced as CVE-2015-8863.