Skip to content

Fix outfilename buffer overflow (account for NUL-termination)#7

Open
jackdroido wants to merge 1 commit intolouigi600:masterfrom
jackdroido:fix-outfilename-buffer-overflow
Open

Fix outfilename buffer overflow (account for NUL-termination)#7
jackdroido wants to merge 1 commit intolouigi600:masterfrom
jackdroido:fix-outfilename-buffer-overflow

Conversation

@jackdroido
Copy link
Copy Markdown

Trying to run my script through obash results in malloc assertion:

./obash -c -r updater.sh
Output filename will be: updater.sh.x
Random uuid: 86e6e59c-dfb7-cbbc-0299-69bc48215e9d
Random serial: 4732cceab9185c45
Used key: >86e6e59cdfb7cbbc029969bc48215e9d<
Used IV: >4732cceab9185c45<
obash: malloc.c:2401: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
Aborted

Found the culprit by looking at valgrind output:

valgrind --leak-check=yes --track-origins=yes -- ./obash -c -r updater.sh
==5334== Memcheck, a memory error detector
==5334== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==5334== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==5334== Command: ./obash -c -r updater.sh
==5334==
Output filename will be: updater.sh.x
Random uuid: c3cfe9b1-e3ae-0d5d-965c-c90d4c2f28de
Random serial: b9d939a1c40d25ab
Used key: >c3cfe9b1e3ae0d5d965cc90d4c2f28de<
Used IV: >b9d939a1c40d25ab<
==5334== Invalid write of size 1
==5334== at 0x109740: mk_sh_c (functions.c:284)
==5334== by 0x10A92A: main (obash.c:240)
==5334== Address 0x4d2871c is 0 bytes after a block of size 12 alloc'd
==5334== at 0x483025B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x109704: mk_sh_c (functions.c:282)
==5334== by 0x10A92A: main (obash.c:240)
==5334==
==5334== Syscall param openat(filename) points to unaddressable byte(s)
==5334== at 0x4C0E26E: open (open.c:44)
==5334== by 0x4B9AF8A: _IO_file_open (fileops.c:189)
==5334== by 0x4B9B15A: _IO_file_fopen@@GLIBC_2.1 (fileops.c:281)
==5334== by 0x4B8E947: __fopen_internal (iofopen.c:78)
==5334== by 0x4B8E9D1: fopen@@GLIBC_2.1 (iofopen.c:89)
==5334== by 0x109755: mk_sh_c (functions.c:287)
==5334== by 0x10A92A: main (obash.c:240)
==5334== Address 0x4d2871c is 0 bytes after a block of size 12 alloc'd
==5334== at 0x483025B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x109704: mk_sh_c (functions.c:282)
==5334== by 0x10A92A: main (obash.c:240)
==5334==
input filename: updater.sh
input file size: 19467
ciphertext size: 19472
base64 encoded ciphertext: 26369 : 405 whole lines
==5334== Invalid read of size 1
==5334== at 0x4833403: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x4B738D1: vfprintf (vfprintf.c:1643)
==5334== by 0x4B79C65: printf (printf.c:33)
==5334== by 0x10991B: mk_sh_c (functions.c:325)
==5334== by 0x10A92A: main (obash.c:240)
==5334== Address 0x4d2871c is 0 bytes after a block of size 12 alloc'd
==5334== at 0x483025B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x109704: mk_sh_c (functions.c:282)
==5334== by 0x10A92A: main (obash.c:240)
==5334==
intermediate c generated filename: updater.sh.c
Creating reusable intermadiate c file
Created updater.sh.c
Compiling updater.sh.c ... done
Cleaning up intermediate c file: updater.sh.c ... done
Output filename: updater.sh.x
==5334==
==5334== HEAP SUMMARY:
==5334== in use at exit: 39,190 bytes in 2 blocks
==5334== total heap usage: 2,665 allocs, 2,663 frees, 301,265 bytes allocated
==5334==
==5334== 256 bytes in 1 blocks are definitely lost in loss record 1 of 2
==5334== at 0x483025B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x10A36D: main (obash.c:152)
==5334==
==5334== 38,934 bytes in 1 blocks are definitely lost in loss record 2 of 2
==5334== at 0x483025B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5334== by 0x1097F8: mk_sh_c (functions.c:304)
==5334== by 0x10A92A: main (obash.c:240)
==5334==
==5334== LEAK SUMMARY:
==5334== definitely lost: 39,190 bytes in 2 blocks
==5334== indirectly lost: 0 bytes in 0 blocks
==5334== possibly lost: 0 bytes in 0 blocks
==5334== still reachable: 0 bytes in 0 blocks
==5334== suppressed: 0 bytes in 0 blocks
==5334==
==5334== For counts of detected and suppressed errors, rerun with: -v
==5334== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0)

malloc() of outfilename doesn't account for the terminating NUL char, and the subsequent strcat() overflow the buffer.

Please note that the bug is not always reproducible since we are in "undefined behaviour land" (see above for an example, under valgrind the overflow is detected but obash doesn't assert out). I could not trigger it with a typical hello world script, but with my script (~20K size) it has.

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

Successfully merging this pull request may close these issues.

1 participant