[Bug] Double free in tcpedit_dlt_cleanup in tcprewrite

[iskindar]

Describe the bug
tcprewrite in tcpreplay latest commit : 43693c4, v4.4.4 and v.4.4.3 has a double free in function tcpedit_dlt_cleanup in plugins/dlt_plugins.c.

To Reproduce
Steps to reproduce the behavior:

1. Get the Tcpreplay source code and build it with ASAN.

# Build with ASAN
export CC=gcc export CXX=g++
export CFLAGS="-g -fsanitize=address" export CXXFLAGS="-g -fsanitize=address"
./autogen.sh
./configure && make -j

2. Run tcprewrite with provided poc

tcprewrite -i poc -o /dev/null

The poc is available at poc.zip

Please unzip it first and then feed it into the tcprewrite binary.

Expected behavior

The ASAN report

==72056==ERROR: AddressSanitizer: attempting double-free on 0x603000000040 in thread T0:
    #0 0x7f10d91ff40f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x555929e48229 in our_safe_free /benchmark/tcpreplay/src/common/utils.c:113
    #2 0x555929e2dbe6 in tcpedit_dlt_cleanup plugins/dlt_plugins.c:466
    #3 0x555929e1dc35 in tcpedit_close /benchmark/tcpreplay/src/tcpedit/tcpedit.c:555
    #4 0x555929e19cd1 in main /benchmark/tcpreplay/src/tcprewrite.c:146
    #5 0x7f10d8e8f082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)
    #6 0x555929e1714d in _start (/validate_binary/tcprewrite+0x1c14d)

0x603000000040 is located 0 bytes inside of 20-byte region [0x603000000040,0x603000000054)
freed by thread T0 here:
    #0 0x7f10d91ff40f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x555929e48229 in our_safe_free /benchmark/tcpreplay/src/common/utils.c:113
    #2 0x555929e2dbe6 in tcpedit_dlt_cleanup plugins/dlt_plugins.c:466
    #3 0x555929e3eceb in dlt_jnpr_ether_cleanup plugins/dlt_jnpr_ether/jnpr_ether.c:168
    #4 0x555929e2dac3 in tcpedit_dlt_cleanup plugins/dlt_plugins.c:450
    #5 0x555929e1dc35 in tcpedit_close /benchmark/tcpreplay/src/tcpedit/tcpedit.c:555
    #6 0x555929e19cd1 in main /benchmark/tcpreplay/src/tcprewrite.c:146
    #7 0x7f10d8e8f082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)

previously allocated by thread T0 here:
[poc.zip](https://github.com/appneta/tcpreplay/files/12090735/poc.zip)

    #0 0x7f10d91ff808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x555929e47f77 in our_safe_malloc /benchmark/tcpreplay/src/common/utils.c:42
    #2 0x555929e2f400 in dlt_en10mb_init plugins/dlt_en10mb/en10mb.c:109
    #3 0x555929e2c804 in tcpedit_dlt_init plugins/dlt_plugins.c:148
    #4 0x555929e3eab4 in dlt_jnpr_ether_post_init plugins/dlt_jnpr_ether/jnpr_ether.c:138
    #5 0x555929e2cf36 in tcpedit_dlt_post_init plugins/dlt_plugins.c:251
    #6 0x555929e2cc41 in tcpedit_dlt_post_args plugins/dlt_plugins.c:202
    #7 0x555929e20842 in tcpedit_post_args /benchmark/tcpreplay/src/tcpedit/parse_args.c:242
    #8 0x555929e1985a in main /benchmark/tcpreplay/src/tcprewrite.c:84
    #9 0x7f10d8e8f082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082)

SUMMARY: AddressSanitizer: double-free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122 in __interceptor_free
==72056==ABORTING

Screenshots

if you reproduce successfully, you will see an output similar to the following screenshot.

System (please complete the following information):

• OS: Ubuntu 20.04 (docker images)
• Tcpreplay Version v4.4.4 and v4.4.3

$ ./tcprewrite --version
tcprewrite version: 4.4.4 (build git:v4.4.4-1-g43693c4a)
Copyright 2013-2022 by Fred Klassen <tcpreplay at appneta dot com> - AppNeta
Copyright 2000-2012 by Aaron Turner <aturner at synfin dot net>
The entire Tcpreplay Suite is licensed under the GPLv3
Cache file supported: 04
Not compiled with libdnet.
Compiled against libpcap: 1.9.1
64 bit packet counters: enabled
Verbose printing via tcpdump: disabled
Fragroute engine: disable

[bsmojver]

@fklassen, it seems that somehow freeing of sub-contexts actually frees something that gets freed again (not that I really understand the code). This particular plugin is calling these sub-context de-allocations, which then get called again.

Are sub-contexts copies of contexts and setting pointers of allocated memory to NULL in these copies isn't reflected in the originals, causing double free?

[carnil]

It looks this got CVE-2023-4256 assigned.

[GabrielGanne]

Hi,

I had a look and it seems that juniper has an exception in the way the plugins works with regard to the extra buffer in question: tcpreplay works with the assumption that there only ever is a single link layer plugin which is mostly true except here: Juniper has a special call to tcpedit_dlt_copy_decoder_state() which causes the ctx and subctx to share a reference to the decoded_extra buffer, and the double call through the backtrace as said in this description indeed causes the issue.

I also note that the plugin architecture is quite nice and should absolutely allow juniper to work as it does. I mean it would be a shame to break it IMHO.
Since each plugin is working with the assumption that it owns the decoded_extra buffer, I suggest to just give each one its own. That would cost a bit more, but not significantly so, and it would also enable the ability to rewrite one link layer into another.

I won't have much time to work on this next month, but I'm willing to work on this after if you want.

Also, this CVE does not seem that bad to me, but if I'm wrong I believe you could use 5ad9d1d701e644ed5b8821456e31acf2e72920c to work around this issue safely.