search

visit1985 / mdp

A command-line based markdown presentation tool.
GNU General Public License v3.0
4.96k stars 257 forks source link

heap-buffer-overflow in mdp #165

Open Frank-Z7 opened 1 year ago

Frank-Z7 commented 1 year ago

heap-buffer-overflow in mdp

Hi.I found a heap-buffer-overflow bug in mdp.

Please confirm.

Thanks for your time！

Version

mdp 1.0.15
Copyright (C) 2018 Michael Goehler
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Michael Goehler and others, see <https://github.com/visit1985/mdp/blob/master/AUTHORS>.

ASAN Log

./mdp -e -i -x poc1mdp

=================================================================
==3175382==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x607000006040 at pc 0x0000004cc70e bp 0x7fffffffe1e0 sp 0x7fffffffe1d8
READ of size 4 at 0x607000006040 thread T0
    #0 0x4cc70d  (/afltest/mdp/mdp+0x4cc70d)
    #1 0x4ccba1  (/afltest/mdp/mdp+0x4ccba1)
    #2 0x4c696c  (/afltest/mdp/mdp+0x4c696c)
    #3 0x7ffff7be9082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)
    #4 0x41d65d  (/afltest/mdp/mdp+0x41d65d)

0x607000006040 is located 0 bytes to the right of 80-byte region [0x607000005ff0,0x607000006040)
allocated by thread T0 here:
    #0 0x4960b9 in realloc (/afltest/mdp/mdp+0x4960b9)
    #1 0x4c5cc1  (/afltest/mdp/mdp+0x4c5cc1)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/afltest/mdp/mdp+0x4cc70d)
Shadow bytes around the buggy address:
  0x0c0e7fff8bb0: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0e7fff8bc0: fd fa fa fa fa fa 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff8bd0: fa fa fa fa fd fd fd fd fd fd fd fd fd fa fa fa
  0x0c0e7fff8be0: fa fa fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c0e7fff8bf0: fd fd fd fd fd fd fd fd fd fa fa fa fa fa 00 00
=>0x0c0e7fff8c00: 00 00 00 00 00 00 00 00[fa]fa fa fa fd fd fd fd
  0x0c0e7fff8c10: fd fd fd fd fd fa fa fa fa fa 00 00 00 00 00 00
  0x0c0e7fff8c20: 00 00 00 00 fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0e7fff8c30: fd fa fa fa fa fa fd fd fd fd fd fd fd fd fd fa
  0x0c0e7fff8c40: fa fa fa fa fd fd fd fd fd fd fd fd fd fa fa fa
  0x0c0e7fff8c50: fa fa fd fd fd fd fd fd fd fd fd fa fa fa fa fa
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
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb
Shadow gap:              cc
==3175382==ABORTING

Reproduction

Steps to reproduce:
1.Download the poc1mdp file.
2.Compile the source code with ASan.
(My approach is to install AFLplusplus and add the following two lines to the Makefile:
CC=afl-clang-fast
CXX=afl-clang-fast++

then:
AFL_USE_ASAN=1 make
)
3.Execute the following command:
./mdp -e -i -x poc1mdp

PoC

poc1mdp: https://github.com/Frank-Z7/z-vulnerabilitys/blob/main/poc1mdp

Impact

This vulnerability is capable of causing crashes, or possible DOS.

Reference

https://github.com/visit1985/mdp

Environment

ubuntu:20.04
gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.2)
clang version 10.0.0-4ubuntu1
afl-cc++4.09
visit1985 commented 11 months ago

Hey @Frank-Z7, thanks for reporting this. Shouldn't this cause a crash? I can't produce one when i follow your steps without afl on

macOS Monterey 12.7.1
Apple clang version 14.0.0 (clang-1400.0.29.202)
Arch Linux
gcc version 12.2.1 20230201 (GCC)
Frank-Z7 commented 11 months ago

Hey @Frank-Z7, thanks for reporting this. Shouldn't this cause a crash? I can't produce one when i follow your steps without afl on

macOS Monterey 12.7.1
Apple clang version 14.0.0 (clang-1400.0.29.202)
Arch Linux
gcc version 12.2.1 20230201 (GCC)

Hi @visit1985 ,

Thank you for your attention. The vulnerability is not related to the use of afl, which comes with the ASAN(AddressSanitizer) tool to check for memory errors.

AddressSanitizer (https://github.com/google/sanitizers), Google famous memory testing tool, it could print out the report and display of memory problem, and has been integrated in the GCC, LLVM compiler, etc.

To make it easier for you to reproduce the problem, I changed the Makefile in the mdp folder and the mdp/src folder. I used the gcc compiler and added the -fsanitize=address flag to enable ASAN. The modified compressed package is as follows. You can directly run the "./ mdp-e-i-x poc1"command to rectify the problem.

Frank-Z7 commented 11 months ago

mdp2.tar.gz

Frank-Z7 commented 11 months ago

I made the following changes to the Makefile: I used the gcc compiler and added the -fsanitize=address flag to enable ASAN to conveniently reveal the heap-buffer-overfkow problem. Thanks for your time.

image image

sudhackar commented 5 months ago

Looking at this - This comes from the assumption that the input would be valid.

This is an Out Of Bound Read when trying to parse with https://github.com/visit1985/mdp/blob/4664cb261145fbd2b164b0ca507af8ceb76ded9c/src/url.c#L168-L172

In here we loop until we see a NULL or ], but then we increment either way. A simple input to repro this is

[  test
extra-line

with these steps on my machine

$ PAGER= git diff
diff --git a/Makefile b/Makefile
index 06f2f83..6e016c5 100644
--- a/Makefile
+++ b/Makefile
@@ -29,8 +29,8 @@ BINDIR  ?= ${PREFIX}/bin
 MANDIR  ?= ${PREFIX}/share/man

 CURSES  = ncursesw
-LDFLAGS ?= -s
-CFLAGS   ?= -O3
+# LDFLAGS ?= -s
+CFLAGS   ?= -O0
 CFLAGS   += -Wall

 ifeq (Windows_NT,$(OS))

$ CC=$(which clang-17) CFLAGS="-g -ggdb3 -fsanitize=address" make

the backtrace is this

$ ASAN_OPTIONS="color=never" ./mdp /tmp/test |& sed -e 's/^[ \t]*//'
=================================================================
==2176893==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5040000000b8 at pc 0x55714789071b bp 0x7ffd81aae320 sp 0x7ffd81aae318
READ of size 4 at 0x5040000000b8 thread T0
#0 0x55714789071a in url_count_inline /dev/shm/mdp/src/url.c:165:12
#1 0x557147891142 in ncurses_display /dev/shm/mdp/src/viewer.c:68:23
#2 0x557147889977 in main /dev/shm/mdp/src/main.c:170:18
#3 0x7ff79ec2814f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#4 0x7ff79ec28208 in __libc_start_main csu/../csu/libc-start.c:360:3
#5 0x5571477ac5c4 in _start (/dev/shm/mdp/mdp+0x305c4) (BuildId: 4c6cbd97323972338cbb256483e51febf8d8669c)

0x5040000000b8 is located 0 bytes after 40-byte region [0x504000000090,0x5040000000b8)
allocated by thread T0 here:
#0 0x557147849c29 in realloc (/dev/shm/mdp/mdp+0xcdc29) (BuildId: 4c6cbd97323972338cbb256483e51febf8d8669c)
#1 0x557147887f78 in cstring_expand /dev/shm/mdp/src/cstring.c:48:27
#2 0x55714788ab2f in markdown_load /dev/shm/mdp/src/parser.c:231:13
#3 0x5571478897be in main /dev/shm/mdp/src/main.c:151:16
#4 0x7ff79ec2814f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /dev/shm/mdp/src/url.c:165:12 in url_count_inline
Shadow bytes around the buggy address:
0x503ffffffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x504000000000: fa fa 00 00 00 00 07 fa fa fa 00 00 00 00 00 fa
=>0x504000000080: fa fa 00 00 00 00 00[fa]fa fa 00 00 00 00 00 fa
0x504000000100: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 00
0x504000000180: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
0x504000000200: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 00 fa
0x504000000280: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 00 fa
0x504000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb
==2176893==ABORTING
Frank-Z7 commented 4 months ago

Looking at this - This comes from the assumption that the input would be valid.

This is an Out Of Bound Read when trying to parse with

https://github.com/visit1985/mdp/blob/4664cb261145fbd2b164b0ca507af8ceb76ded9c/src/url.c#L168-L172

In here we loop until we see a NULL or ], but then we increment either way. A simple input to repro this is

[  test
extra-line

with these steps on my machine

$ PAGER= git diff
diff --git a/Makefile b/Makefile
index 06f2f83..6e016c5 100644
--- a/Makefile
+++ b/Makefile
@@ -29,8 +29,8 @@ BINDIR  ?= ${PREFIX}/bin
 MANDIR  ?= ${PREFIX}/share/man

 CURSES  = ncursesw
-LDFLAGS ?= -s
-CFLAGS   ?= -O3
+# LDFLAGS ?= -s
+CFLAGS   ?= -O0
 CFLAGS   += -Wall

 ifeq (Windows_NT,$(OS))

$ CC=$(which clang-17) CFLAGS="-g -ggdb3 -fsanitize=address" make

the backtrace is this

$ ASAN_OPTIONS="color=never" ./mdp /tmp/test |& sed -e 's/^[ \t]*//'
=================================================================
==2176893==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5040000000b8 at pc 0x55714789071b bp 0x7ffd81aae320 sp 0x7ffd81aae318
READ of size 4 at 0x5040000000b8 thread T0
#0 0x55714789071a in url_count_inline /dev/shm/mdp/src/url.c:165:12
#1 0x557147891142 in ncurses_display /dev/shm/mdp/src/viewer.c:68:23
#2 0x557147889977 in main /dev/shm/mdp/src/main.c:170:18
#3 0x7ff79ec2814f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#4 0x7ff79ec28208 in __libc_start_main csu/../csu/libc-start.c:360:3
#5 0x5571477ac5c4 in _start (/dev/shm/mdp/mdp+0x305c4) (BuildId: 4c6cbd97323972338cbb256483e51febf8d8669c)

0x5040000000b8 is located 0 bytes after 40-byte region [0x504000000090,0x5040000000b8)
allocated by thread T0 here:
#0 0x557147849c29 in realloc (/dev/shm/mdp/mdp+0xcdc29) (BuildId: 4c6cbd97323972338cbb256483e51febf8d8669c)
#1 0x557147887f78 in cstring_expand /dev/shm/mdp/src/cstring.c:48:27
#2 0x55714788ab2f in markdown_load /dev/shm/mdp/src/parser.c:231:13
#3 0x5571478897be in main /dev/shm/mdp/src/main.c:151:16
#4 0x7ff79ec2814f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /dev/shm/mdp/src/url.c:165:12 in url_count_inline
Shadow bytes around the buggy address:
0x503ffffffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x503fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x504000000000: fa fa 00 00 00 00 07 fa fa fa 00 00 00 00 00 fa
=>0x504000000080: fa fa 00 00 00 00 00[fa]fa fa 00 00 00 00 00 fa
0x504000000100: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 00
0x504000000180: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd
0x504000000200: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 00 fa
0x504000000280: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 00 fa
0x504000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb
==2176893==ABORTING

Sorry I just saw your reply, thank you for your supplement and analysis! I agree with your point of view, "-g -ggdb3" makes ASAN log clear

Frank-Z7 commented 4 months ago

I would like to report a cve based on this issue. Do you mind if I include your name as a contributor? @sudhackar

sudhackar commented 4 months ago

Hello @Frank-Z7

I don't have any issue whatsoever. How are you applying for a CVE? We should get this fixed before we get a CVE - I can think of a patch. I'll try to raise a PR later.

Frank-Z7 commented 4 months ago

Hello @Frank-Z7

I don't have any issue whatsoever. How are you applying for a CVE? We should get this fixed before we get a CVE - I can think of a patch. I'll try to raise a PR later.

Hi~The url_len_inline function seems to have the same problem. Perhaps we can limit it based on cstring_t->alloc?