invalid memory read in queuepush.c / function queue_push()

hannob commented 8 years ago

Compiling yodl with address sanitizer (-fsanitize=address) shows an invalid memory read in the function queue_push().

I tried to look at the source and find the bug, but I'm not familiar with the code base and was unable to easily determine the reason.

This can be reproduced simply by trying to compile everything with address sanitizer enabled: CFLAGS="-fsanitize=address -g" LDFLAGS="-fsanitize=address" ./build programs CFLAGS="-fsanitize=address -g" LDFLAGS="-fsanitize=address" ./build macros CFLAGS="-fsanitize=address -g" LDFLAGS="-fsanitize=address" ./build man

This was tested with release 3.05.01. The error message from address sanitizer:

==19388==ERROR: AddressSanitizer: unknown-crash on address 0x61400000ee40 at pc 0x418d47 bp 0x7ffe39342bc0 sp 0x7ffe39342bb0
READ of size 613 at 0x61400000ee40 thread T0
    #0 0x418d46 in queue_push /tmp/yodl-3.05.01/src/queue/queuepush.c:51
    #1 0x41436d in lexer_push_str /tmp/yodl-3.05.01/src/lexer/lexerpushstr.c:28
    #2 0x41c6b0 in p_expand_macro /tmp/yodl-3.05.01/src/parser/pexpandmacro.c:51
    #3 0x41c0d7 in p_default_symbol /tmp/yodl-3.05.01/src/parser/pdefaultesymbol.c:20
    #4 0x4167b3 in p_handle_default_symbol /tmp/yodl-3.05.01/src/parser/phandledefaultsymbol.c:5
    #5 0x40dd26 in p_parse /tmp/yodl-3.05.01/src/parser/pparse.c:18
    #6 0x40cbe6 in parser_process /tmp/yodl-3.05.01/src/parser/parserprocess.c:39
    #7 0x407e5a in main /tmp/yodl-3.05.01/src/yodl/yodl.c:14
    #8 0x7f0ed56d761f in __libc_start_main (/lib64/libc.so.6+0x2061f)
    #9 0x401e28 in _start (/tmp/yodl-3.05.01/tmp/install/usr/bin/yodl+0x401e28)

0x61400000efd7 is located 0 bytes to the right of 407-byte region [0x61400000ee40,0x61400000efd7)
allocated by thread T0 here:
    #0 0x7f0ed5aab7d7 in malloc (/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/libasan.so.1+0x577d7)
    #1 0x409c4b in n_malloc /tmp/yodl-3.05.01/src/new/nmalloc.c:11
    #2 0x418533 in new_memory ../new/new.h:42
    #3 0x4185e1 in queue_construct /tmp/yodl-3.05.01/src/queue/queueconstruct.c:11
    #4 0x41499b in l_media_construct_memory /tmp/yodl-3.05.01/src/lexer/lmediaconstructmemory.c:9
    #5 0x4150a8 in l_push /tmp/yodl-3.05.01/src/lexer/lpush.c:15
    #6 0x414171 in lexer_push_str /tmp/yodl-3.05.01/src/lexer/lexerpushstr.c:20
    #7 0x41c6b0 in p_expand_macro /tmp/yodl-3.05.01/src/parser/pexpandmacro.c:51
    #8 0x41c0d7 in p_default_symbol /tmp/yodl-3.05.01/src/parser/pdefaultesymbol.c:20
    #9 0x4167b3 in p_handle_default_symbol /tmp/yodl-3.05.01/src/parser/phandledefaultsymbol.c:5
    #10 0x40dd26 in p_parse /tmp/yodl-3.05.01/src/parser/pparse.c:18
    #11 0x40cbe6 in parser_process /tmp/yodl-3.05.01/src/parser/parserprocess.c:39
    #12 0x407e5a in main /tmp/yodl-3.05.01/src/yodl/yodl.c:14
    #13 0x7f0ed56d761f in __libc_start_main (/lib64/libc.so.6+0x2061f)

SUMMARY: AddressSanitizer: unknown-crash /tmp/yodl-3.05.01/src/queue/queuepush.c:51 queue_push
Shadow bytes around the buggy address:
  0x0c287fff9d70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
  0x0c287fff9d80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c287fff9d90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fff9da0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fff9db0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
=>0x0c287fff9dc0: fa fa fa fa fa fa fa fa[00]00 00 00 00 00 00 00
  0x0c287fff9dd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff9de0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff9df0: 00 00 00 00 00 00 00 00 00 00 07 fa fa fa fa fa
  0x0c287fff9e00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c287fff9e10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==19388==ABORTING

fbb-git commented 8 years ago

Dear Hanno B??ck, you wrote:

Compiling yodl with address sanitizer (-fsanitize=address) shows an invalid memory read in the function queue_push().

Thanks for the bug report. I'll check it out asap.

Cheers,

Frank B. Brokken
Center for Information Technology, University of Groningen
(+31) 50 363 9281 
Public PGP key: http://pgp.surfnet.nl
Key Fingerprint: DF32 13DE B156 7732 E65E  3B4D 7DB2 A8BE EAE4 D8AA

fbb-git commented 8 years ago

Dear Hanno B??ck, you wrote:

Compiling yodl with address sanitizer (-fsanitize=address) shows an invalid memory read in the function queue_push().

The problem is caused by copying too many bytes from an existing queue to the enlarged queue.

To fix the problem (it will be fixed in Yodl's next release) you can apply the following patch in the directory src/queue:

--- queuepush.c 2016-02-04 21:59:48.694823071 +0100 +++ /tmp/queuepush.c 2016-02-04 21:59:43.154817125 +0100 @@ -29,8 +29,11 @@

 if (extra_length > available_length)
 {

```
 size_t original_length = memory_length;
```
+ /* enlarge the buffer: */ memory_length += extra_length - available_length + BLOCK_QUEUE; + cp = new_memory(memory_length, sizeof(char));
```
if (message_show(MSG_INFO))
```
@@ -48,7 +51,7 @@ } else /* q as one block */ {
memcpy(cp, qp->d_memory, memory_length);/* cp existing buffer */
memcpy(cp, qp->d_memory, original_length);/* cp existing buffer */ qp->d_read = cp + (qp->d_read - qp->d_memory); qp->d_write = cp + (qp->d_write - qp->d_memory); }

(Saving this file as '/tmp/patch' do 'patch -p0 < /tmp/patch').

I also noticed that the address sanitizer reported several memory leaks. Not nice, and not the way it should be, but probably harder to fix. It'll probably be a while before I've fixed those leaks.

Thanks again for your bug-report!

Frank B. Brokken
Center for Information Technology, University of Groningen
(+31) 50 363 9281 
Public PGP key: http://pgp.surfnet.nl
Key Fingerprint: DF32 13DE B156 7732 E65E  3B4D 7DB2 A8BE EAE4 D8AA

andreasstieger commented 7 years ago

Invalid memory read addressed by https://github.com/fbb-git/yodl/commit/fd85f8c94182558ff1480d06a236d6fb927979a3

msmeissn commented 7 years ago

cve requested via webform

msmeissn commented 7 years ago

CVE-2016-10375

fbb-git commented 7 years ago

Dear Marcus Meissner, you wrote:

CVE-2016-10375

Thanks for your posts about Yodl. Maybe you can provide a bit more info, like what the problem is you observed, and preferably also a yodl file producing the problem you observed?

I just compiled yodl using Hannob's (Feb 4, 2016) suggestion and got some memory leaks:

================================================================= ==29971==ERROR: LeakSanitizer: detected memory leaks

These are known and will be dealt with in due time (I'll try to squash them in Yodl's upcoming new version, which might be a major upgrade. Working on that right now, but I'm unable to give a realistic estimate as to when it will be available).

-- Frank B. Brokken Center for Information Technology, University of Groningen (+31) 50 363 9281 Public PGP key: http://pgp.surfnet.nl Key Fingerprint: DF32 13DE B156 7732 E65E 3B4D 7DB2 A8BE EAE4 D8AA

fgeek commented 7 years ago

CVE-identifiers are assigned for security vulnerabilities:

https://en.wikipedia.org/wiki/Common_Vulnerabilities_and_Exposures https://cve.mitre.org/

fbb-git commented 7 years ago

The memory leaks reported by Hannob on Feb 4, 2016 have been dealth with in Yodl version 3.10.00, which I just uploaded to github. Since the memory read error in queue_push was dealt with in an earlier update (version 3.07.01), the issue reported by Hannob has now been dealt with and therefore I'm closing the issue.

msmeissn commented 7 years ago

(Just for the record ... CVE are dictionary entries that reference single security issues and their fixes. the CVE listed is for this specific issue with invalid memory read that you fixed in 3.07.01).

all good. :)

fbb-git commented 7 years ago

Dear Marcus Meissner, you wrote:

(Just for the record ... CVE are dictionary entries that reference single security issues and their fixes. the CVE listed is for this specific issue with invalid memory read that you fixed in 3.07.01).

all good. :)

OK, thx!

-- Frank B. Brokken Center for Information Technology, University of Groningen (+31) 50 363 9281 Public PGP key: http://pgp.surfnet.nl Key Fingerprint: DF32 13DE B156 7732 E65E 3B4D 7DB2 A8BE EAE4 D8AA

fbb-git / yodl

invalid memory read in queuepush.c / function queue_push() #1