An empty file is able to cause an out of bounds read: # touch foo # paps foo ================================================================= ==30527==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000dfaf at pc 0x0000004e122d bp 0x7ffd8f3dfe90 sp 0x7ffd8f3dfe88 READ of size 1 at 0x60200000dfaf thread T0 #0 0x4e122c in read_file /tmp/portage/app-text/paps-0.6.8-r1/work/paps-0.6.8/src/paps.c:573:7 #1 0x4e122c in main /tmp/portage/app-text/paps-0.6.8-r1/work/paps-0.6.8/src/paps.c:493 #2 0x7fd8aff707af in __libc_start_main (/lib64/libc.so.6+0x207af) #3 0x436968 in _start (/usr/bin/paps+0x436968) 0x60200000dfaf is located 1 bytes to the left of 4-byte region [0x60200000dfb0,0x60200000dfb4) allocated by thread T0 here: #0 0x4bdc75 in realloc (/usr/bin/paps+0x4bdc75) #1 0x7fd8b111c35d in g_realloc (/usr/lib64/libglib-2.0.so.0+0x4e35d) SUMMARY: AddressSanitizer: heap-buffer-overflow /tmp/portage/app-text/paps-0.6.8-r1/work/paps-0.6.8/src/paps.c:573 read_file Shadow bytes around the buggy address: 0x0c047fff9ba0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9bb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9be0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c047fff9bf0: fa fa fa fa fa[fa]04 fa fa fa 00 02 fa fa 00 02 0x0c047fff9c00: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa 0x0c047fff9c10: fa fa fd fa fa fa 00 00 fa fa 00 00 fa fa 00 00 0x0c047fff9c20: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa fd fa 0x0c047fff9c30: fa fa fd fa fa fa fd fa fa fa 00 00 fa fa 00 00 0x0c047fff9c40: fa fa 00 fa fa fa 00 00 fa fa 00 00 fa fa 00 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 Heap right redzone: fb Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 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 ==30527==ABORTING This behavior does not happen if the file is not empty.
(In reply to Agostino Sarubbo from comment #0) > An empty file is able to cause an out of bounds read: After more tests, the crash happen also if the file is not empty and contains random data.
Hi Ago, With regards to the empty file, this is likely fixed with a trivial patch like this: diff -ru paps-0.6.8/src/paps.c paps-0.6.8-modified/src/paps.c --- paps-0.6.8/src/paps.c 2007-04-13 07:04:14.000000000 +0200 +++ paps-0.6.8-modified/src/paps.c 2016-07-12 15:42:25.182937820 +0200 @@ -569,11 +569,14 @@ fclose (file); - /* Add a trailing new line if it is missing */ - if (inbuf->str[inbuf->len-1] != '\n') - g_string_append(inbuf, "\n"); + if (inbuf->len) { + /* Add a trailing new line if it is missing */ + if (inbuf->str[inbuf->len-1] != '\n') + g_string_append(inbuf, "\n"); - text = inbuf->str; + text = inbuf->str; + } else + text = g_strdup("\n"); g_string_free (inbuf, FALSE); return text; This patch hasn't been tested, but it probably does what we want. The out of bounds read itself is likely not dangerous, unless the rest of the program relies on the \n and that buffer can somehow be massaged into having or not having the \n. Nonetheless, it's a bug with undefined behavior and should probably be fixed. The other crashes you mentioned -- the ones with random data -- that's much more worrying. Can you give the ASAN backtrace for these?
(In reply to Jason A. Donenfeld from comment #2) > The other crashes you mentioned -- the ones with random data -- that's much > more worrying. Can you give the ASAN backtrace for these? Thanks to have looked into it. The behavior and the ASAN backtrace are the same if the file is empty or contains random data.
The patch seems to cover both cases.
Good to hear. Have you contacted upstream about this?
(In reply to Jason A. Donenfeld from comment #5) > Good to hear. Have you contacted upstream about this? Yes, no response since 1 year. Also, the upstream website in not anymore reachable and latest release was ~2006.
Adding maintainer for comment. Since it is a low probability of this bug being exploitable, also removing restriction on the bug.
(In reply to Jason A. Donenfeld from comment #2) > diff -ru paps-0.6.8/src/paps.c paps-0.6.8-modified/src/paps.c thanks (In reply to Agostino Sarubbo from comment #6) > (In reply to Jason A. Donenfeld from comment #5) > ... Also, the upstream website in not anymore > reachable ... http://paps.sourceforge.net/ is available. I add aforementioned patch as 0.6.8-r2.
commit a04f61b7bbfe1e82cf00e9354fc5d6f3b389f719 Author: Michael Weber <xmw@gentoo.org> Date: Mon Jul 25 17:23:10 2016 +0200 app-text/paps: Fix bad handling of empty files (bug 566050).
@arches, please stabilize: =app-text/paps-0.6.8-r2
amd64 stable
x86 stable. Maintainer(s), please cleanup. Security, please vote.
commit 8b028d99a3785fecdc54d8774d907ecbdb1aa93b Author: Michael Weber <xmw@gentoo.org> Date: Fri Jul 29 18:43:09 2016 +0200 app-text/paps: drop old revision (bug 566050).
GLSA Vote: No
