Bug 63576 (CVE-2004-1304)

<!-- SBZ_reproduce  -->
-

We have a lot of fixes and cahnges included in readelf.c.
Therefore I'd like to see a exploit before I'm going
to inverstigate all file versions back to 8.1

An not-working exploit proofs nothing. 
 

But looking at the 8.1 code I know what you mean. ;> There are a lot of 
changes made since. 
 
How can I get access to their CVS? Didnt found anything on www.gnu.org. 

No CVS at all, only the source tar ball at

    ftp://ftp.astron.com/pub/file/

That is what I am curently working with...  

The question: Which of the diff between 4.10 and 4.12 _is_vulnerable
and if my patch could also avoid an attack.  This because I've
fixed some segmentation faults, see bug 55909 and #41209.

I've submited 4.12 to STABLE

Created attachment 26516 [details]
foo.pat

from vendor-sec, the fix for the elf loader

actually more a question than a patch... still waiting here for more info from 
vendor-sec 

Yep IMHO in the attachent of comment #9 there is the
possible crahs. But note that I'm using since

     Mon May 24 12:25:30 CEST 2004

the following piece of code:

        if (offset + descsz >= size) {
               /*
                * We're past the end of the buffer.
                */
               return (offset >= size) ? offset : size;
        }

to be sure not to be fooled.  Beside this I've added this
check `(offset >= size) ? offset : size' on several other
places.

Back from illness. 
 
I contacted the authors of the various patches over the weekend. 
 
Result will be attached later today.  

> 2004 10-12 10:50  Christos Zoulas  <christos@zoulas.com> 
> 
>         * Fix stack overwriting on 0 length strings: Tim Waugh 
>             <twaugh@redhat.com> Ned Ludd <solar@gentoo.org> 
 
--- Initial mail as sent to christos@zoulas.com -- 
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] = '\0'; 
n = strlen(p->s) - 1; 
if (p->s[n] == '\n') 
        p->s[n] = '\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. 
-------------------------------------------------- 
 

Created attachment 26576 [details]
file-4.10-mconvert.patch

> 2004 10-12 10:50  Christos Zoulas  <christos@zoulas.com>
>
>	  * Fix stack overwriting on 0 length strings: Tim Waugh
>	      <twaugh@redhat.com> Ned Ludd <solar@gentoo.org>

> 2004-11-24 12:39  Christos Zoulas  <christos@zoulas.com> 
> 
>         * Stack smash fix, 
 
http://cvs.pld-linux.org/cgi-bin/cvsweb/SOURCES/Attic/file-segv.patch?rev=1.1 
 
This bug was introduced in file 4.11, simple testcase could be a file 
with just three bytes: "dd\n". 
 
> and ELF more conservative reading. 
 
http://cvs.pld-linux.org/cgi-bin/cvsweb/SOURCES/Attic/file-readelf-fix.patch?rev=1.3 
 
In fact it contains remaining parts of old version of this patch, which 
weren't been applied when I sent it for the first time to Christos 
(probably were lost during merging with other updates). 
 
>           Jakub Bogusz <qboosh@pld-linux.org> 
> 
> 2004-07-24 16:33  Christos Zoulas  <christos@zoulas.com> 
> 
>         * errors reading elf magic Jakub Bogusz <qboosh@pld-linux.org> 
 
Original version for file 4.07: 
http://cvs.pld-linux.org/cgi-bin/cvsweb/SOURCES/Attic/file-readelf-fix.patch?rev=1.1 
and updated for file 4.08: 
http://cvs.pld-linux.org/cgi-bin/cvsweb/SOURCES/Attic/file-readelf-fix.patch?rev=1.2 
 

> 2004-09-27 11:30  Christos Zoulas  <christos@zoulas.com> 
> 
>         * Fix small memory leaks; caught by: Tamas Sarlos 
>             <stamas@csillag.ilab.sztaki.hu> 
> 
> Thank you. 
> 
 
Here it is, though the leak was rather harmless, since it occured only 
once per magic_open calls. 
 
stamas@d5:~$ diff -rc file-4.10 file-4.10-memleak-fix 
diff -rc file-4.10/src/file.c file-4.10-memleak-fix/src/file.c 
*** file-4.10/src/file.c        Wed Apr  7 16:23:55 2004 
--- file-4.10-memleak-fix/src/file.c    Mon Sep 27 15:30:08 2004 
*************** 
*** 313,318 **** 
--- 313,319 ---- 
                } 
                for (; optind < argc; optind++) 
                        process(argv[optind], wid); 
+               magic_close(magic); 
        } 
 
        return 0; 
diff -rc file-4.10/src/magic.c file-4.10-memleak-fix/src/magic.c 
*** file-4.10/src/magic.c       Sat Jul 24 21:55:17 2004 
--- file-4.10-memleak-fix/src/magic.c   Mon Sep 27 15:46:18 2004 
*************** 
*** 141,146 **** 
--- 141,147 ---- 
  { 
        free_mlist(ms->mlist); 
        free(ms->o.buf); 
+         free(ms->o.pbuf); 
        free(ms->c.off); 
        free(ms); 
  } 
stamas@d5:~$ 
 
Even with the patch above the file program is not fully valgrind clean, 
since magic_close() is not invoked when exiting on errors with exit(1), 
but I don't think it matters. 
 
Gruesse, 
        Tamas 
 

file 4.11 was never part of any SuSE Linux

ok, so at least 
 
> 2004-11-24 12:39  Christos Zoulas  <christos@zoulas.com>  
>  
>         * Stack smash fix 
 
doesnt hurt us. 

Created attachment 26577 [details]
file-stringop.patch

> 2004 10-12 10:50  Christos Zoulas  <christos@zoulas.com>
>
>	  * Fix stack overwriting on 0 length strings: Tim Waugh
>	      <twaugh@redhat.com> Ned Ludd <solar@gentoo.org>


file-4.10-mconvert.patch is incomplete

The question is: Do we need an security update?

Created attachment 26580 [details]
file-4.07-elfmagic.patch

Created attachment 26581 [details]
file-4.08-elfmagic.patch

Created attachment 26582 [details]
file-4.10-memleak.patch

Created attachment 26583 [details]
file-4.11-stacksmash.patch

Created attachment 26584 [details]
file-elfreading.patch

Now which version have to be fixed?

werner/file> ll /work/SRC/old-versions/8.1/UL/all/file \
? /work/SRC/old-versions/9.0/all/file \
? /work/SRC/old-versions/9.1/SLES/all/file \
? /work/SRC/old-versions/9.2/all/file \
? | grep tar.bz2
-rw-r--r--  3 root root 142864 Mar  4  2003 file-3.37.tar.bz2
-rw-r--r--  2 root root 284214 May 24  2003 file-4.03.tar.bz2
-rw-r--r--  1 root root 295499 Dec 23  2003 file-4.07.tar.bz2
-rw-r--r--  2 root root 303110 Apr  7  2004 file-4.09.tar.bz2

IMHO the patch file-4.07-elfmagic.patch is wrong, see file-4.12

Created attachment 26602 [details]
Security update for file-3.37

3.37 and 4.x are clearly different, nevertheless something
like that should catch that what I've identified.

Created attachment 26612 [details]
Security update for file-3.37

Created attachment 26613 [details]
Security update for file-4.03

Created attachment 26614 [details]
Security update for file-4.07

Created attachment 26615 [details]
Security update for file-4.09

The memory leak I've ignored, no need to fix that.

Q: Should I submit the packages for SL 8.1/9.0/9.1/9.1 and SLES9?

Yes... I'll do the patchinfo file and SWAMP stuff tomorrow. 

Ok, all we fix then would be the one byte buffer overflow, right? 
 
If so, let's fix it for stable only. 

STABLE uses 4.12, btw: I've all package ready from 8.1 upto 9.2,
that mean the patch, rpm spec and mbuild are done.  Therefore,
why not submit them?

:) Ok, then let's go. Thank you. 

Done, even the new file package for SLES9-SP2 is fixed.

<!-- SBZ_reopen -->Reopened by thomas@suse.de at Fri Dec  3 17:46:43 2004

thanks! 
 
reopned for tracking 

packages approved 

CVE-2004-1304

CVE-2004-1304: CVSS v2 Base Score: 10.0 (AV:N/AC:L/Au:N/C:C/I:C/A:C)