Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

heap-buffer-overflow in mdp #165

Open
Frank-Z7 opened this issue Nov 9, 2023 · 9 comments
Open

heap-buffer-overflow in mdp #165

Frank-Z7 opened this issue Nov 9, 2023 · 9 comments

Comments

@Frank-Z7
Copy link

Frank-Z7 commented Nov 9, 2023 •
edited
Loading

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
Copy link
Owner

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
Copy link
Author

Frank-Z7 commented Dec 2, 2023

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
Copy link
Author

Frank-Z7 commented Dec 2, 2023

mdp2.tar.gz

@Frank-Z7
Copy link
Author

Frank-Z7 commented Dec 2, 2023

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
Copy link

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

mdp/src/url.c

Lines 168 to 172 in 4664cb2

} else if ( *i == '[' && *(i+1) && *(i+1) != ']') {
while (*i && *i != ']') i++;
i++;
if (*i == '(' && wcschr(i, ')')) {
count ++;

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
Copy link
Author

Frank-Z7 commented Jul 2, 2024

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

mdp/src/url.c

Lines 168 to 172 in 4664cb2

} else if ( *i == '[' && *(i+1) && *(i+1) != ']') {
while (*i && *i != ']') i++;
i++;
if (*i == '(' && wcschr(i, ')')) {
count ++;

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
Copy link
Author

Frank-Z7 commented Jul 2, 2024

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

@sudhackar
Copy link

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
Copy link
Author

Frank-Z7 commented Jul 3, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@visit1985 @sudhackar @Frank-Z7