solar@simple c $ cat main.c #include <stdio.h> #include <stdlib.h> int main(int argc, char **argv, char **envp, char **auxv) { printf("main=%p(argc=%p, argv=%p, envp=%p, auxv=%p)\n", &main, &argc, &argv, &envp, &auxv); system("cat /proc/self/maps"); puts(__PRETTY_FUNCTION__); while(1) { sleep(1); } _exit(EXIT_SUCCESS); } solar@simple c $ make main cc main.o -o main solar@simple c $ objcopy -O binary -R .note -R .comment -S main main.x while [ $? = 0 ] ; do ./file main.x ;done main.x: data main.x: data main.x: data Segmentation fault (core dumped) solar@simple c $ file -v file-4.10 magic file from /usr/share/misc/file/magic solar@simple file $ file /bin/ls Bounds Checking GCC v gcc-3.4.2-3.2 Copyright (C) 1995 Richard W.M. Jones Bounds Checking comes with ABSOLUTELY NO WARRANTY. For details see file `COPYING' that should have come with the source to this program. Bounds Checking is free software, and you are welcome to redistribute it under certain conditions. See the file `COPYING' for details. For more information, set GCC_BOUNDS_OPTS to `-help' softmagic.c:435:Bounds error: array reference (-1) outside bounds of the array. softmagic.c:435: Pointer value: 0xbffecd87 softmagic.c:435: Object `p': softmagic.c:435: Address in memory: 0xbffecd88 .. 0xbffecda7 softmagic.c:435: Size: 32 bytes softmagic.c:435: Element size: 1 bytes softmagic.c:435: Number of elements: 32 softmagic.c:435: Created at: softmagic.c, line 111 softmagic.c:435: Storage class: stack /bin/ls: Aborted (core dumped) (gdb) solar@simple file $ gdb -q `which file` core Using host libthread_db library "/lib/libthread_db.so.1". Core was generated by `file /bin/ls'. Program terminated with signal 6, Aborted. Reading symbols from /usr/lib/libmagic.so.1...done. Loaded symbols for /usr/lib/libmagic.so.1 Reading symbols from /lib/libz.so.1...done. Loaded symbols for /lib/libz.so.1 Reading symbols from /lib/libc.so.6...done. Loaded symbols for /lib/libc.so.6 Reading symbols from /lib/ld-linux.so.2...done. Loaded symbols for /lib/ld-linux.so.2 #0 0x40087811 in kill () from /lib/libc.so.6 (gdb) bt full #0 0x40087811 in kill () from /lib/libc.so.6 No symbol table info available. #1 0x400874a3 in raise () from /lib/libc.so.6 No symbol table info available. #2 0x4008886e in abort () from /lib/libc.so.6 No symbol table info available. #3 0x08052b56 in __bounds_errorf (filename=0x40040725 "softmagic.c", line=435, pointer=0x0, obj=0x80744c0, format=0x0) at /space/portage-tmp/portage/gcc-3.4.2-r2/work/gcc-3.4.2/gcc/bounds/lib/error.c:200 temp = (void *) 0xbffecd87 #4 0x0804b8bc in __bounds_check_array_reference_obj (pointer=0xbffecd88, offset=-1, size=1, array_size=134629192, filename=0x40040725 "softmagic.c", line=435) at /space/portage-tmp/portage/gcc-3.4.2-r2/work/gcc-3.4.2/gcc/bounds/lib/check.c:228 savebase = (void *) 0xbffecd88 saveextent = (void *) 0xbffecda8 savesize = 32 savealign = 111 #5 0x400358aa in mget (ms=0x80746c0, p=0xbffecd88, s=0xbffed170 "\177ELF\001\001\001", m=0x4015ac80, nbytes=65536) at softmagic.c:435 offset = 58 #6 0x40037652 in file_softmagic (ms=0x80746c0, buf=0xbffed170 "\177ELF\001\001\001", nbytes=65536) at softmagic.c:121 ml = (struct mlist *) 0x8077fc0 #7 0x4003eecf in file_buffer (ms=0x80746c0, buf=0xbffed170, nb=65536) at funcs.c:123 m = 0 #8 0x4002e3b5 in magic_file (ms=0x80746c0, inname=0xbfffe7a5 "/bin/ls") at magic.c:281 fd = 3 buf = "\177ELF\001\001\001\000\000\000\000\000\000\000\000\000\003\000\003\000\001\000\000\000 #\000\0004\000\000\000L
solar@simple c $ cat main.c #include <stdio.h> #include <stdlib.h> int main(int argc, char **argv, char **envp, char **auxv) { printf("main=%p(argc=%p, argv=%p, envp=%p, auxv=%p)\n", &main, &argc, &argv, &envp, &auxv); system("cat /proc/self/maps"); puts(__PRETTY_FUNCTION__); while(1) { sleep(1); } _exit(EXIT_SUCCESS); } solar@simple c $ make main cc main.o -o main solar@simple c $ objcopy -O binary -R .note -R .comment -S main main.x while [ $? = 0 ] ; do ./file main.x ;done main.x: data main.x: data main.x: data Segmentation fault (core dumped) solar@simple c $ file -v file-4.10 magic file from /usr/share/misc/file/magic solar@simple file $ file /bin/ls Bounds Checking GCC v gcc-3.4.2-3.2 Copyright (C) 1995 Richard W.M. Jones Bounds Checking comes with ABSOLUTELY NO WARRANTY. For details see file `COPYING' that should have come with the source to this program. Bounds Checking is free software, and you are welcome to redistribute it under certain conditions. See the file `COPYING' for details. For more information, set GCC_BOUNDS_OPTS to `-help' softmagic.c:435:Bounds error: array reference (-1) outside bounds of the array. softmagic.c:435: Pointer value: 0xbffecd87 softmagic.c:435: Object `p': softmagic.c:435: Address in memory: 0xbffecd88 .. 0xbffecda7 softmagic.c:435: Size: 32 bytes softmagic.c:435: Element size: 1 bytes softmagic.c:435: Number of elements: 32 softmagic.c:435: Created at: softmagic.c, line 111 softmagic.c:435: Storage class: stack /bin/ls: Aborted (core dumped) (gdb) solar@simple file $ gdb -q `which file` core Using host libthread_db library "/lib/libthread_db.so.1". Core was generated by `file /bin/ls'. Program terminated with signal 6, Aborted. Reading symbols from /usr/lib/libmagic.so.1...done. Loaded symbols for /usr/lib/libmagic.so.1 Reading symbols from /lib/libz.so.1...done. Loaded symbols for /lib/libz.so.1 Reading symbols from /lib/libc.so.6...done. Loaded symbols for /lib/libc.so.6 Reading symbols from /lib/ld-linux.so.2...done. Loaded symbols for /lib/ld-linux.so.2 #0 0x40087811 in kill () from /lib/libc.so.6 (gdb) bt full #0 0x40087811 in kill () from /lib/libc.so.6 No symbol table info available. #1 0x400874a3 in raise () from /lib/libc.so.6 No symbol table info available. #2 0x4008886e in abort () from /lib/libc.so.6 No symbol table info available. #3 0x08052b56 in __bounds_errorf (filename=0x40040725 "softmagic.c", line=435, pointer=0x0, obj=0x80744c0, format=0x0) at /space/portage-tmp/portage/gcc-3.4.2-r2/work/gcc-3.4.2/gcc/bounds/lib/error.c:200 temp = (void *) 0xbffecd87 #4 0x0804b8bc in __bounds_check_array_reference_obj (pointer=0xbffecd88, offset=-1, size=1, array_size=134629192, filename=0x40040725 "softmagic.c", line=435) at /space/portage-tmp/portage/gcc-3.4.2-r2/work/gcc-3.4.2/gcc/bounds/lib/check.c:228 savebase = (void *) 0xbffecd88 saveextent = (void *) 0xbffecda8 savesize = 32 savealign = 111 #5 0x400358aa in mget (ms=0x80746c0, p=0xbffecd88, s=0xbffed170 "\177ELF\001\001\001", m=0x4015ac80, nbytes=65536) at softmagic.c:435 offset = 58 #6 0x40037652 in file_softmagic (ms=0x80746c0, buf=0xbffed170 "\177ELF\001\001\001", nbytes=65536) at softmagic.c:121 ml = (struct mlist *) 0x8077fc0 #7 0x4003eecf in file_buffer (ms=0x80746c0, buf=0xbffed170, nb=65536) at funcs.c:123 m = 0 #8 0x4002e3b5 in magic_file (ms=0x80746c0, inname=0xbfffe7a5 "/bin/ls") at magic.c:281 fd = 3 buf = "\177ELF\001\001\001\000\000\000\000\000\000\000\000\000\003\000\003\000\001\000\000\000 #\000\0004\000\000\000LÀ\001\000\000\000\000\0004\000 \000\n\000(\000\033\000\032\000\006\000\000\0004\000\000\0004\000\000\0004\000\000\000@\001\000\000@\001\000\000\005\000\000\000\004\000\000\000\003\000\000\000t\001\000\000t\001\000\000t\001\000\000\023\000\000\000\023\000\000\000\004\000\000\000\001\000\000\000\001", '\0' <repeats 15 times>, "T \001\000T \001\000\005\000\000\000\000\020\000\000\001\000\000\000,©\001\000,¹\001\000,¹\001\000Ô\a\000\000\204\v\000\000\006\000\000\000\000\020\000\000\002\000\000\000 \001\000 ½\001\000 ½\001\000è\000\000\000è\000\000\000\006"... sb = {st_dev = 2054, __pad1 = 0, __st_ino = 34158, st_mode = 33261, st_nlink = 1, st_uid = 0, st_gid = 0, st_rdev = 0, __pad2 = 0, st_size = 115844, st_blksize = 4096, st_blocks = 240, st_atim = {tv_sec = 1100279851, tv_nsec = 0}, st_mtim = {tv_sec = 1092948296, tv_nsec = 0}, st_ctim = {tv_sec = 1092948296, tv_nsec = 0}, st_ino = 34158} nbytes = 65536 #9 0x08049c07 in process (inname=0xbfffe7a5 "/bin/ls", wid=0) at file.c:388 type = 0xbfffe7a5 "/bin/ls" std_in = 0 #10 0x0804a548 in main (argc=2, argv=0xbfffe654) at file.c:315 i = 2 wid = 7 nw = 0
Created attachment 43793 [details] softmagic.c cat -n src/softmagic.c | grep " 111" | head -n1 111 union VALUETYPE p;
Lines 428-438 case FILE_STRING: { int n; /* Null terminate and eat *trailing* return */ p->s[sizeof(p->s) - 1] = '\0'; n = strlen(p->s) - 1; if (p->s[n] == '\n') p->s[n] = '\0'; return 1; }
simple src # diff -u softmagic.c.orig softmagic.c --- softmagic.c.orig 2004-11-12 12:52:15.000000000 -0500 +++ softmagic.c 2004-11-12 12:44:45.000000000 -0500 @@ -427,10 +427,12 @@ return 1; case FILE_STRING: { + #include <assert.h> int n; /* Null terminate and eat *trailing* return */ p->s[sizeof(p->s) - 1] = '\0'; + assert (strlen(p->s) > 0); n = strlen(p->s) - 1; if (p->s[n] == '\n') p->s[n] = '\0'; simple file-4.10 # src/file /bin/ls lt-file: softmagic.c:435: mconvert: Assertion `strlen(p->s) > 0' failed. /bin/ls: Aborted
Maybe patch? simple file-4.10 # diff -u src/softmagic.c.orig src/softmagic.c --- src/softmagic.c.orig 2004-11-12 12:52:15.000000000 -0500 +++ src/softmagic.c 2004-11-12 12:55:54.000000000 -0500 @@ -432,7 +432,7 @@ /* Null terminate and eat *trailing* return */ p->s[sizeof(p->s) - 1] = '\0'; n = strlen(p->s) - 1; - if (p->s[n] == '\n') + if (n && p->s[n] == '\n') p->s[n] = '\0'; return 1; }
--- src/softmagic.c.orig 2004-11-12 13:30:22.000000000 -0500 working version. --- src/softmagic.c.orig 2004-11-12 13:30:22.000000000 -0500 +++ src/softmagic.c 2004-11-12 13:32:27.000000000 -0500 @@ -432,7 +432,7 @@ /* Null terminate and eat *trailing* return */ p->s[sizeof(p->s) - 1] = '\0'; n = strlen(p->s) - 1; - if (p->s[n] == '\n') + if ((n > 0) && (p->s[n] == '\n')) p->s[n] = '\0'; return 1; }
simple file-4.10 # src/file /bin/ls Bounds Checking GCC v gcc-3.4.2-3.2 Copyright (C) 1995 Richard W.M. Jones Bounds Checking comes with ABSOLUTELY NO WARRANTY. For details see file `COPYING' that should have come with the source to this program. Bounds Checking is free software, and you are welcome to redistribute it under certain conditions. See the file `COPYING' for details. For more information, set GCC_BOUNDS_OPTS to `-help' /bin/ls: ELF 32-bit LSB shared object, Intel 80386, version 1 (SYSV), stripped Bounds library call frequency statistics: Calls to push, pop, param function: 1643, 1643, 529 Calls to add, delete stack: 1252, 1252 Calls to add, delete heap: 11, 4 Calls to check pointer +/- integer: 3799 Calls to check array references: 1615 Calls to check pointer differences: 2 Calls to check object references: 2538 Calls to check component references: 9320 Calls to check truth, falsity of pointers: 6, 0 Calls to check <, >, <=, >= of pointers: 0 Calls to check ==, != of pointers: 25 Calls to check p++, ++p, p--, --p: 1277, 0, 0, 0 Calls to add, find, delete oob pointers: 0, 0, 0 References to unchecked static, stack: 0, 0
GCC_BOUNDS_OPTS="-no-message -no-statistics" src/file /bin/ls /bin/ls: ELF 32-bit LSB shared object, Intel 80386, version 1 (SYSV), stripped
x=0 ; while [ $? = 0 ] && [ $x -le 1024 ] ; do x=$(($x + 1)) ; GCC_BOUNDS_OPTS="-no-message -no-statistics" src/file /c/main.x > /dev/null ; done No error.
Looks like the same thing can happen again later in line 449 --- src/softmagic.c.orig 2004-11-12 13:30:22.000000000 -0500 +++ src/softmagic.c 2004-11-12 13:47:55.000000000 -0500 @@ -432,7 +432,7 @@ /* Null terminate and eat *trailing* return */ p->s[sizeof(p->s) - 1] = '\0'; n = strlen(p->s) - 1; - if (p->s[n] == '\n') + if ((n > 0) && (p->s[n] == '\n')) p->s[n] = '\0'; return 1; } @@ -446,7 +446,7 @@ *ptr1++ = *ptr2++; *ptr1 = '\0'; n = strlen(p->s) - 1; - if (p->s[n] == '\n') + if ((n > 0) && (p->s[n] == '\n')) p->s[n] = '\0'; return 1; }
Auditing Team please confirm my findings.
I have no idea if that can be exploited. Usually if a program pushes data onto the stack then PaX usually lets me know and with 'file' I see no such thing. Being an underflow I'm not sure. probably just a bug.
underflows can be exploitable if you can hit some variable that's used later and lets you influence the program behaviour. whether that's the case here or not i can't tell, judging from the sigsegv you can do something, but i'm not too keen on debugging this myself, after all 'file' isn't privileged per se, so you could at most exploit an unsuspecting root spying on your own files ;-). as for the suggested fix, while it's ok at the low C level, i don't know if it's correct from the program's logic point of view, the original author should be asked about it (e.g., you may have to return some error or do some other processing upon this n==0 condition).
Created attachment 43850 [details, diff] file-4.10-mconvert.patch From: Ned Ludd <solar/gentoo.org> Reply-To: solar/gentoo.org To: Christos Zoulas <christos/zoulas.com> Subject: Re: gentoo-bug-70975 - sys-apps/file-4.10-r1 - oob on stack in function mconvert() Date: Sat, 13 Nov 2004 03:58:23 -0500 On Sat, 2004-11-13 at 02:42, Christos Zoulas wrote: > On Nov 13, 1:03am, solar/gentoo.org (Ned Ludd) wrote: > -- Subject: gentoo-bug-70975 - sys-apps/file-4.10-r1 - oob on stack in functi > > | I've identified a stack underflow in atleast file-4.10 maybe older > | versions. I was unable to reproduce the problem with an older file-4.02 > | that I found on mirrors but the same code is present. > | > | Here is what I believe to be the area of code which is problematic. > | > | p->s[sizeof(p->s) - 1] =3D '\0'; > | n =3D strlen(p->s) - 1; > | if (p->s[n] =3D=3D '\n') > | p->s[n] =3D '\0'; > | > | If strlen(p->s) equals 0 and you subtract 1 > | then p->s[-1] underflows on the union. > | > | I would like for you to confirm these findings and determine if you > | think it could be a security problem or not. I've attached further > | debugging details which confirm these findings and a patch which > | mitigates the problem at hand. > > I don't think it is a security problem because you can only overwrite one > byte on the stack with 0. Well, the patch is partially correct. The > second instance is unsigned so the test will not work. > > my code looks like now: > > size_t n; > ... > n = strlen(p->s); > if (n-- && p->s[n] == '\n') > p->s[n] = '\0'; Thanks for the fast response an improved fix. passes gcc qa tests. Being that it's not seen as a security problem by you and assuming you do not to notice anything else that should fixed within the next 12 hours I'll put the attached patch based your Re: to Gentoos unstable ~arch tree for testing. > Thanks a lot for letting me know, > > christos On another note From looking over our build script for file it seems our mips arch adds the following patch for reasons unknown to me and seem they have been continually doing so from 4.05. Perhaps it's never been shown to you for review.. http://www.gentoo.org/cgi-bin/viewcvs.cgi/*checkout*/sys-apps/file/files/file-4.xx-mips-gentoo.diff?rev=HEAD&content-type=text/plain
Bug deemed not a security problem by the author. file-4.10-mconvert.patch added to file-4.10-r1 Closing bug as FIXED.
