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.

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

unit-test-server crashes with invalid pointer #750

Closed
balckgu1 opened this issue May 29, 2024 · 4 comments
Closed

unit-test-server crashes with invalid pointer #750

balckgu1 opened this issue May 29, 2024 · 4 comments

Comments

@balckgu1
Copy link


libmodbus version

libmodbus v3.1.6

OS and/or distribution

Ubuntu 18

Environment

..

Description

A vulnerability has been identified in libmodbus v3.1.6, which can be triggered by sending a specific message to unit-test-server. The vulnerability appears to be due to the modbus_receive() function incorrectly manipulating a query pointer or out-of-bounds array after executing malloc() to allocate memory, causing the program to crash with an invalid pointer at free(query).

Actual behavior if applicable

free(): invalid pointer

Expected behavior or suggestion

no crash

Steps to reproduce the behavior (commands or source code)

  1. Add the -fsanitize=address and -g parameter at compile time
  ./autogen.sh
  ./configure --enable-static CC="clang -fsanitize=address  -O1 -g" CXX="clang -fsanitize=address  -O1 -g"
   make -j8
   cd tests
   clang -g -fsanitize=address -O1  unit-test-server.c -o unit-test-server -I ../src/ ../src/.libs/libmodbus.a
  1. send POC
    POC:
    POC.zip

libmodbus output with debug mode enabled

The location of the crash can be further determined by debugging with gdb

  (gdb) run < /POC
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tests/unit-test-server < /POC
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff7bff640 (LWP 18712)]
[New Thread 0x7ffff73fe640 (LWP 18713)]
[Thread 0x7ffff7bff640 (LWP 18712) exited]
The client connection from 0.0.0.0 is accepted
Waiting for an indication...
<03><DD><00><00><00><0D><FF><16><01><62><00><01><00><84>
[03][DD][00][00][00][08][FF][16][01][62][00][01][00][84]
Waiting for an indication...
<03><DD><00><00><00><0D><FF><17><01><62><00><01><00><84><00><01><02><00><01>
[03][DD][00][00][00][05][FF][17][02][00][84]
Waiting for an indication...
ERROR Connection reset by peer: read
<02>Quit the loop: Connection reset by peer
free(): invalid pointer
�b����
Thread 1 "unit-test-serve" received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140737353619264) at ./nptl/pthread_kill.c:44
44	./nptl/pthread_kill.c: No such file or directory.

(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140737353619264)
    at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140737353619264)
    at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140737353619264, signo=signo@entry=6)
    at ./nptl/pthread_kill.c:89
#3  0x00007ffff7c42476 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/posix/raise.c:26
#4  0x00007ffff7c287f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007ffff7c89676 in __libc_message (action=action@entry=do_abort, 
    fmt=fmt@entry=0x7ffff7ddbb77 "%s\n") at ../sysdeps/posix/libc_fatal.c:155
#6  0x00007ffff7ca0cfc in malloc_printerr (
    str=str@entry=0x7ffff7dd9744 "free(): invalid pointer")
    at ./malloc/malloc.c:5664
#7  0x00007ffff7ca2a44 in _int_free (av=<optimized out>, p=<optimized out>, 
    have_lock=0) at ./malloc/malloc.c:4439
#8  0x00007ffff7ca5453 in __GI___libc_free (mem=<optimized out>)
    at ./malloc/malloc.c:3391
#9  0x0000555555556ae3 in main (argc=1, argv=0x7fffffffdba8)
    at unit-test-server.c:197

Put breakpoints in unit-test-server.c wherever query is used and debug again

(gdb) break unit-test-server.c:115
Breakpoint 4 at 0x55555555675a: file unit-test-server.c, line 115.
(gdb) break unit-test-server.c:130
Breakpoint 5 at 0x5555555567a9: file unit-test-server.c, line 130.
(gdb) break unit-test-server.c:133
Breakpoint 6 at 0x5555555567ea: file unit-test-server.c, line 133.
(gdb) break unit-test-server.c:135
Breakpoint 7 at 0x55555555680f: file unit-test-server.c, line 135.
(gdb) break unit-test-server.c:138
Breakpoint 8 at 0x555555556857: file unit-test-server.c, line 138.
(gdb) break unit-test-server.c:141
Breakpoint 9 at 0x55555555686e: file unit-test-server.c, line 141.
(gdb) break unit-test-server.c:153
Breakpoint 10 at 0x5555555568f5: file unit-test-server.c, line 153.
(gdb) break unit-test-server.c:157
Breakpoint 11 at 0x555555556942: file unit-test-server.c, line 157.
(gdb) break unit-test-server.c:170
Breakpoint 12 at 0x5555555569ca: file unit-test-server.c, line 170.
(gdb) break unit-test-server.c:183
Breakpoint 13 at 0x555555556a68: file unit-test-server.c, line 183.
(gdb) i b
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x0000555555556429 in main at unit-test-server.c:34
	breakpoint already hit 1 time
2       breakpoint     keep y   0x00005555555564ec in main at unit-test-server.c:59
	breakpoint already hit 1 time
3       breakpoint     keep y   0x0000555555556ad1 in main at unit-test-server.c:196
	breakpoint already hit 1 time
4       breakpoint     keep y   0x000055555555675a in main at unit-test-server.c:115
5       breakpoint     keep y   0x00005555555567a9 in main at unit-test-server.c:130
6       breakpoint     keep y   0x00005555555567ea in main at unit-test-server.c:133
7       breakpoint     keep y   0x000055555555680f in main at unit-test-server.c:135
8       breakpoint     keep y   0x0000555555556857 in main at unit-test-server.c:138
9       breakpoint     keep y   0x000055555555686e in main at unit-test-server.c:141
10      breakpoint     keep y   0x00005555555568f5 in main at unit-test-server.c:153
11      breakpoint     keep y   0x0000555555556942 in main at unit-test-server.c:157
12      breakpoint     keep y   0x00005555555569ca in main at unit-test-server.c:170
13      breakpoint     keep y   0x0000555555556a68 in main at unit-test-server.c:183

After the modbus_receive(), the value of the pointer changed, and it was this change that caused the crash at free(query).

gdb) p query
$18 = (uint8_t *) 0x555555561320 <incomplete sequence \335>
(gdb) Quit

Continuing the trace, it turns out that it ends up in the recv()

(gdb) run < /home/zyl/libmodbus-afl-gcc-exp/libmodbus/tests/outputs_afl_two_sta_1/crashes/id:000001,sig:06,src:000169,op:arith8,pos:27,val:+31
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /home/zyl/libmodbus-3.1.6/libmodbus/tests/unit-test-server < /home/zyl/libmodbus-afl-gcc-exp/libmodbus/tests/outputs_afl_two_sta_1/crashes/id:000001,sig:06,src:000169,op:arith8,pos:27,val:+31
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, main (argc=1, argv=0x7fffffffdba8) at unit-test-server.c:34
34	    int s = -1;
(gdb) c
Continuing.

Breakpoint 2, main (argc=1, argv=0x7fffffffdba8) at unit-test-server.c:59
59	    if (use_backend == TCP) {
(gdb) c
Continuing.
[New Thread 0x7ffff7bff640 (LWP 18993)]
[New Thread 0x7ffff73fe640 (LWP 18994)]
The client connection from 0.0.0.0 is accepted

Thread 1 "unit-test-serve" hit Breakpoint 4, main (argc=1, argv=0x7fffffffdba8) at unit-test-server.c:115
115	            rc = modbus_receive(ctx, query);
(gdb) s
[Thread 0x7ffff7bff640 (LWP 18993) exited]

Thread 1 "unit-test-serve" hit Breakpoint 24, modbus_receive (ctx=0x5555555612a0, req=0x555555561320 "") at modbus.c:484
484	{
(gdb) c
Continuing.
Waiting for an indication...

Thread 1 "unit-test-serve" hit Breakpoint 25, _modbus_receive_msg (ctx=0x5555555612a0, msg=0x555555561320 "", msg_type=MSG_INDICATION) at modbus.c:405
405	        rc = ctx->backend->recv(ctx, msg + msg_length, length_to_read);
(gdb) s
_modbus_tcp_recv (ctx=0x5555555612a0, rsp=0x555555561320 "", rsp_length=8) at modbus-tcp.c:180
180	    return recv(ctx->s, (char *)rsp, rsp_length, 0);
(gdb) s
__libc_recv (fd=4, buf=0x555555561320, len=8, flags=0) at ../sysdeps/unix/sysv/linux/recv.c:24

I'm guessing that there could be an array out of bounds due to insufficient memory allocated by the malloc?

@sandeen
Copy link

sandeen commented Jun 10, 2024

I'm trying to help out with this because I package libmodbus for Fedora and EPEL, but I'll be honest, I'm not quite sure how the test server works. I was trying to reproduce the crash directly, without starting off in gdb.

If I do this on Fedora rawhide:

# ./configure --enable-static CC="clang -fsanitize=address  -O1 -g" CXX="clang -fsanitize=address  -O1 -g"
# make
# cd tests/
# clang -g -fsanitize=address -O1  unit-test-server.c -o unit-test-server -I ../src/ ../src/.libs/libmodbus.a
# wget https://github.com/stephane/libmodbus/files/15481104/POC.zip
# unzip POC.zip
# ./unit-test-server < ./id\:000001\,sig\:06\,src\:000169\,op\:arith8\,pos\:27\,val\:+31

... nothing happens. Should it?

@BrunoVernay
Copy link

@psychon
Copy link

psychon commented Jul 11, 2024

Same result as with your other report. This is a duplicate of #748

  • I can reproduce the problem with version 3.1.6.
  • I cannot reproduce with master
  • git bisect says this was fixed years ago (edit: in version 3.1.7)
b4ef4c17d618eba0adccc4c7d9e9a1ef809fc9b6 is the first bad commit
commit b4ef4c17d618eba0adccc4c7d9e9a1ef809fc9b6
Author: Michael Heimpold <mhei@heimpold.de>
Date:   Sat Jan 8 20:00:50 2022 +0100

    modbus_reply: fix copy & paste error in sanity check (fixes #614)
    
    While handling MODBUS_FC_WRITE_AND_READ_REGISTERS, both address offsets
    must be checked, i.e. the read and the write address must be within the
    mapping range.
    
    At the moment, only the read address was considered, it looks like a
    simple copy and paste error, so let's fix it.
    
    Signed-off-by: Michael Heimpold <mhei@heimpold.de>

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

I suggest to just close the issue.

@stephane
Copy link
Owner

Thank you @psychon for the bisect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants