A heap-use-after-free bug find in unit-test-server.c (V3.1.6)

[Rrooach]

Hello, I found a heap-use-after-free bug in stephane/libmodbus/unit-test-server.c.

Below are Steps Followed to Reproduce Crash
I'm using Ubuntu16.04. Download latest source code from: stephane/libmodbus/unit-test-server.c, compiled with clang and ASANexport CFLAGS="-g -fsanitize=address" LDFLAGS="-fsanitize=address"before make. Then I fuzz it with afl, in order to make your program runnable with afl, I use peeny to reload system's socket function by adding prefix LD_PRELOAD=/preeny/x86_64-linux-gnu/desock.so. Also I made few adjustments in your program, more specifically, I modified the loop for(;;) into if(1), and the break/continue statements within the loop, used to break the for(;;) was changed into goto, which allows it jumps out the code block as break/continue did.(Same modification as #498)

Input Data
crash.zip

Crash Output

The client connection from 0.0.0.0 is accepted
Waiting for an indication...
<00><00><00><00><00><0D><FF><17><01><60><00><01><01><00><00><01><02><12><34>
=================================================================
==13121==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000000020 at pc 0x7fbda7fc4952 bp 0x7ffcd407b250 sp 0x7ffcd407b248
WRITE of size 2 at 0x606000000020 thread T0                                                                                                                                                                            #0 0x7fbda7fc4951 in modbus_reply /root/temp/iec/untest/libmodbus/src/modbus.c:980:46
    #1 0x511ddd in main /root/temp/iec/untest/libmodbus/tests/unit-test-server.c:183:14
    #2 0x7fbda70c382f in __libc_start_main /build/glibc-LK5gWL/glibc-2.23/csu/../csu/libc-start.c:291
    #3 0x41a168 in _start (/root/temp/iec/untest/libmodbus/tests/unit-test-server+0x41a168)

0x606000000020 is located 0 bytes inside of 52-byte region [0x606000000020,0x606000000054)                                                                                                                         freed by thread T0 here:                                                                                                                                                                                               #0 0x4daaa0 in __interceptor_cfree.localalias.0 (/root/temp/iec/untest/libmodbus/tests/unit-test-server+0x4daaa0)
    #1 0x7fbda7684654 in _dlerror_run /build/glibc-LK5gWL/glibc-2.23/dlfcn/dlerror.c:159

previously allocated by thread T0 here:                                                                                                                                                                                #0 0x4dac68 in __interceptor_malloc (/root/temp/iec/untest/libmodbus/tests/unit-test-server+0x4dac68)
    #1 0x7fbda8421340 in _dl_signal_error /build/glibc-LK5gWL/glibc-2.23/elf/dl-error.c:90

SUMMARY: AddressSanitizer: heap-use-after-free /root/temp/iec/untest/libmodbus/src/modbus.c:980:46 in modbus_reply
Shadow bytes around the buggy address:
0x0c0c7fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c0c7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c0c7fff8000: fa fa fa fa[fd]fd fd fd fd fd fd fa fa fa fa fa                                                                                                                                                    0x0c0c7fff8010: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00                                                                                                                                                    0x0c0c7fff8020: 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa                                                                                                                                                    0x0c0c7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa                                                                                                                                                    0x0c0c7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa                                                                                                                                                    0x0c0c7fff8050: 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                                                                                                                                                                                      ==13121==ABORTING

[jsbosch]

Did you intend while(1) instead of if(1)? The former loops (equivalent to for (;;)) while the latter executes once.

Also, why add goto? Those should only be used if there's absolutely no other way to branch.

[Rrooach]

Did you intend while(1) instead of if(1)? The former loops (equivalent to for (;;)) while the latter executes once.

Also, why add goto? Those should only be used if there's absolutely no other way to branch.

because the origin program was designed to send/receive packets constantly, while the AFL with preeny can only receive packets once at a time, in order to enable AFL test Modbus, we then make the above adaptation.