Debian Bug report logs - #308624
gdb: [CAN-2005-1704] Integer overflow in ELF segment parsing

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Moritz Muehlenhoff <jmm@inutil.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: gdb: Integer overflow in ELF segment parsing

Date: Wed, 11 May 2005 16:29:30 +0200

Package: gdb
Version: 6.3-5
Severity: grave
Tags: security
Justification: user security hole

An integer overflow in parsing ELF segment headers has been found that
can potentially be exploited to corrupt the heap and execute arbitrary
code. See http://bugs.gentoo.org/show_bug.cgi?id=91398 for a crafted
test binary (without malicious effects) and pointers to more information.

SuSE supposedly has prepared a patch, but I couldn't find it yet.

Cheers,
        Moritz

-- System Information:
Debian Release: 3.1
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.6.11
Locale: LANG=C, LC_CTYPE=de_DE.ISO-8859-15@euro (charmap=ISO-8859-15)

Versions of packages gdb depends on:
ii  libc6                       2.3.2.ds1-21 GNU C Library: Shared libraries an
ii  libncurses5                 5.4-4        Shared libraries for terminal hand
ii  libreadline4                4.3-15       GNU readline and history libraries

-- no debconf information

Message #10 received at 308624@bugs.debian.org (full text, mbox, reply):

From: Matthijs Mohlmann <matthijs@cacholong.nl>

To: 308624@bugs.debian.org

Subject: Re: gdb: Integer overflow in ELF segment parsing

Date: Sat, 21 May 2005 00:31:50 +0200

[Message part 1 (text/plain, inline)]

Hi,

Here a patch that fixes the problem. (It works for me) Got it from the
current snapshot. I think it needs some more testing but AFAICS it's fixed.

Regards,

Matthijs Mohlmann

[security.fix (text/plain, inline)]

diff -ruN gdb-6.3.orig/bfd/elfcode.h gdb-6.3/bfd/elfcode.h
--- gdb-6.3.orig/bfd/elfcode.h	2004-07-27 16:20:47.000000000 +0200
+++ gdb-6.3/bfd/elfcode.h	2005-05-21 00:04:45.493158472 +0200
@@ -613,8 +613,13 @@
 
   if (i_ehdrp->e_shoff != 0)
     {
+     bfd_signed_vma where = i_ehdrp->e_shoff;
+
+     if (where != (file_ptr) where)
+       goto got_wrong_format_error;
+
       /* Seek to the section header table in the file.  */
-      if (bfd_seek (abfd, (file_ptr) i_ehdrp->e_shoff, SEEK_SET) != 0)
+      if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
 	goto got_no_match;
 
       /* Read the first section header at index 0, and convert to internal
@@ -626,12 +631,48 @@
       /* If the section count is zero, the actual count is in the first
 	 section header.  */
       if (i_ehdrp->e_shnum == SHN_UNDEF)
-	i_ehdrp->e_shnum = i_shdr.sh_size;
+       {
+         i_ehdrp->e_shnum = i_shdr.sh_size;
+         if (i_ehdrp->e_shnum != i_shdr.sh_size)
+           goto got_wrong_format_error;
+       }
 
       /* And similarly for the string table index.  */
       if (i_ehdrp->e_shstrndx == SHN_XINDEX)
-	i_ehdrp->e_shstrndx = i_shdr.sh_link;
-    }
+       {
+         i_ehdrp->e_shstrndx = i_shdr.sh_link;
+         if (i_ehdrp->e_shstrndx != i_shdr.sh_link)
+           goto got_wrong_format_error;
+       }
+
+      /* Sanity check that we can read all of the section headers.
+        It ought to be good enough to just read the last one.  */
+      if (i_ehdrp->e_shnum != 1)
+       {
+         /* Check that we don't have a totally silly number of sections.  */
+         if (i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (x_shdr))
+           goto got_wrong_format_error;
+
+         where += (i_ehdrp->e_shnum - 1) * sizeof (x_shdr);
+         if (where != (file_ptr) where)
+           goto got_wrong_format_error;
+         if ((bfd_size_type) where <= i_ehdrp->e_shoff)
+           goto got_wrong_format_error;
+
+         if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+           goto got_no_match;
+         if (bfd_bread (&x_shdr, sizeof x_shdr, abfd) != sizeof (x_shdr))
+           goto got_no_match;
+
+         /* Back to where we were.  */
+         where = i_ehdrp->e_shoff + sizeof (x_shdr);
+         if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+           goto got_no_match;
+       }
+    }
+  /* A further sanity check.  */
+  if (i_ehdrp->e_shstrndx >= i_ehdrp->e_shnum)
+    goto got_wrong_format_error;
 
   /* Allocate space for a copy of the section header table in
      internal form.  */

[signature.asc (application/pgp-signature, attachment)]

Message #17 received at 308624@bugs.debian.org (full text, mbox, reply):

From: Daniel Jacobowitz <drow@false.org>

To: Matthijs Mohlmann <matthijs@cacholong.nl>, 308624@bugs.debian.org

Subject: Re: Patch for security bug in gdb

Date: Fri, 20 May 2005 22:46:11 -0400

On Sat, May 21, 2005 at 12:48:12AM +0200, Matthijs Mohlmann wrote:
> Hi,
> 
> Attached to this mail a patch which fixes a security problem in gdb.
> 
> I tested the patch and it works. Patch comes from the current snapshot
> of gdb, I backported it.
> 
> Bug #308624

There are at least two other patches in CVS; they should all be brought
in together.


-- 
Daniel Jacobowitz
CodeSourcery, LLC

Message #22 received at 308624@bugs.debian.org (full text, mbox, reply):

From: Matthijs Mohlmann <matthijs@cacholong.nl>

To: Daniel Jacobowitz <drow@false.org>

Cc: 308624@bugs.debian.org

Subject: Re: Patch for security bug in gdb

Date: Sat, 21 May 2005 17:30:45 +0200

[Message part 1 (text/plain, inline)]

Hi,

Daniel Jacobowitz wrote:
> On Sat, May 21, 2005 at 12:48:12AM +0200, Matthijs Mohlmann wrote:
> 
>>Hi,
>>
>>Attached to this mail a patch which fixes a security problem in gdb.
>>
>>I tested the patch and it works. Patch comes from the current snapshot
>>of gdb, I backported it.
>>
>>Bug #308624
> 
> 
> There are at least two other patches in CVS; they should all be brought
> in together.
> 
> 

Ok, i have looked further and i think that these patches should help.
This is the cvs log that fixes it:

* elf.c (_bfd_elf_write_object_contents): Check for non-NULL elf_shstrtab.
* format.c (bfd_check_format_matches): Set output_has_begun for
both_direction.
* section.c (bfd_set_section_contents): Use bfd_write_p.  Remove special
case for both_direction.

And for elfcode.h
* elfcode.h (elf_object_p): Add more sanity checks on elf header.

I hope that this helps.

Regards,

Matthijs Mohlmann

[security2.patch (text/x-patch, inline)]

===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.67
retrieving revision 1.68
diff -u -r1.67 -r1.68
--- src/bfd/elfcode.h	2005/05/04 15:53:28	1.67
+++ src/bfd/elfcode.h	2005/05/09 03:35:38	1.68
@@ -33,7 +33,7 @@
 /* Problems and other issues to resolve.
 
    (1)	BFD expects there to be some fixed number of "sections" in
-        the object file.  I.E. there is a "section_count" variable in the
+	the object file.  I.E. there is a "section_count" variable in the
 	bfd structure which contains the number of sections.  However, ELF
 	supports multiple "views" of a file.  In particular, with current
 	implementations, executable files typically have two tables, a
@@ -612,8 +612,13 @@
 
   if (i_ehdrp->e_shoff != 0)
     {
+      bfd_signed_vma where = i_ehdrp->e_shoff;
+
+      if (where != (file_ptr) where)
+	goto got_wrong_format_error;
+
       /* Seek to the section header table in the file.  */
-      if (bfd_seek (abfd, (file_ptr) i_ehdrp->e_shoff, SEEK_SET) != 0)
+      if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
 	goto got_no_match;
 
       /* Read the first section header at index 0, and convert to internal
@@ -625,13 +630,50 @@
       /* If the section count is zero, the actual count is in the first
 	 section header.  */
       if (i_ehdrp->e_shnum == SHN_UNDEF)
-	i_ehdrp->e_shnum = i_shdr.sh_size;
+	{
+	  i_ehdrp->e_shnum = i_shdr.sh_size;
+	  if (i_ehdrp->e_shnum != i_shdr.sh_size)
+	    goto got_wrong_format_error;
+	}
 
       /* And similarly for the string table index.  */
       if (i_ehdrp->e_shstrndx == SHN_XINDEX)
-	i_ehdrp->e_shstrndx = i_shdr.sh_link;
+	{
+	  i_ehdrp->e_shstrndx = i_shdr.sh_link;
+	  if (i_ehdrp->e_shstrndx != i_shdr.sh_link)
+	    goto got_wrong_format_error;
+	}
+
+      /* Sanity check that we can read all of the section headers.
+	 It ought to be good enough to just read the last one.  */
+      if (i_ehdrp->e_shnum != 1)
+	{
+	  /* Check that we don't have a totally silly number of sections.  */
+	  if (i_ehdrp->e_shnum > (unsigned int) -1 / sizeof (x_shdr))
+	    goto got_wrong_format_error;
+
+	  where += (i_ehdrp->e_shnum - 1) * sizeof (x_shdr);
+	  if (where != (file_ptr) where)
+	    goto got_wrong_format_error;
+	  if ((bfd_size_type) where <= i_ehdrp->e_shoff)
+	    goto got_wrong_format_error;
+
+	  if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+	    goto got_no_match;
+	  if (bfd_bread (&x_shdr, sizeof x_shdr, abfd) != sizeof (x_shdr))
+	    goto got_no_match;
+
+	  /* Back to where we were.  */
+	  where = i_ehdrp->e_shoff + sizeof (x_shdr);
+	  if (bfd_seek (abfd, (file_ptr) where, SEEK_SET) != 0)
+	    goto got_no_match;
+	}
     }
 
+  /* A further sanity check.  */
+  if (i_ehdrp->e_shstrndx >= i_ehdrp->e_shnum)
+    goto got_wrong_format_error;
+
   /* Allocate space for a copy of the section header table in
      internal form.  */
   if (i_ehdrp->e_shnum != 0)
@@ -1042,7 +1084,7 @@
 	     symcount);
 
 	  /* Slurp in the symbols without the version information,
-             since that is more helpful than just quitting.  */
+	     since that is more helpful than just quitting.  */
 	  verhdr = NULL;
 	}
 
@@ -1107,7 +1149,7 @@
 	    sym->symbol.section = bfd_abs_section_ptr;
 
 	  /* If this is a relocatable file, then the symbol value is
-             already section relative.  */
+	     already section relative.  */
 	  if ((abfd->flags & (EXEC_P | DYNAMIC)) != 0)
 	    sym->symbol.value -= sym->symbol.section->vma;
 
===================================================================
RCS file: /cvs/src/src/bfd/format.c,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -r1.20 -r1.21
--- src/bfd/format.c	2005/05/04 15:53:31	1.20
+++ src/bfd/format.c	2005/05/17 19:44:55	1.21
@@ -173,6 +173,14 @@
 	  if (matching)
 	    free (matching_vector);
 
+	  /* If the file was opened for update, then `output_has_begun'
+	     some time ago when the file was created.  Do not recompute
+	     sections sizes or alignments in _bfd_set_section_contents.
+	     We can not set this flag until after checking the format,
+	     because it will interfere with creation of BFD sections.  */
+	  if (abfd->direction == both_direction)
+	    abfd->output_has_begun = TRUE;
+
 	  return TRUE;			/* File position has moved, BTW.  */
 	}
 
@@ -319,6 +327,14 @@
       if (matching)
 	free (matching_vector);
 
+      /* If the file was opened for update, then `output_has_begun'
+	 some time ago when the file was created.  Do not recompute
+	 sections sizes or alignments in _bfd_set_section_contents.
+	 We can not set this flag until after checking the format,
+	 because it will interfere with creation of BFD sections.  */
+      if (abfd->direction == both_direction)
+	abfd->output_has_begun = TRUE;
+
       return TRUE;			/* File position has moved, BTW.  */
     }
 
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.294
retrieving revision 1.295
diff -u -r1.294 -r1.295
--- src/bfd/elf.c	2005/05/17 18:08:08	1.294
+++ src/bfd/elf.c	2005/05/17 19:44:55	1.295
@@ -4950,8 +4950,9 @@
     }
 
   /* Write out the section header names.  */
-  if (bfd_seek (abfd, elf_tdata (abfd)->shstrtab_hdr.sh_offset, SEEK_SET) != 0
-      || ! _bfd_elf_strtab_emit (abfd, elf_shstrtab (abfd)))
+  if (elf_shstrtab (abfd) != NULL
+      && (bfd_seek (abfd, elf_tdata (abfd)->shstrtab_hdr.sh_offset, SEEK_SET) != 0
+          || ! _bfd_elf_strtab_emit (abfd, elf_shstrtab (abfd))))
     return FALSE;
 
   if (bed->elf_backend_final_write_processing)
===================================================================
RCS file: /cvs/src/src/bfd/section.c,v
retrieving revision 1.87
retrieving revision 1.88
diff -u -r1.87 -r1.88
--- src/bfd/section.c	2005/05/05 14:34:04	1.87
+++ src/bfd/section.c	2005/05/17 19:44:55	1.88
@@ -1346,22 +1346,10 @@
       return FALSE;
     }
 
-  switch (abfd->direction)
+  if (!bfd_write_p (abfd))
     {
-    case read_direction:
-    case no_direction:
       bfd_set_error (bfd_error_invalid_operation);
       return FALSE;
-
-    case write_direction:
-      break;
-
-    case both_direction:
-      /* File is opened for update. `output_has_begun' some time ago when
-	   the file was created.  Do not recompute sections sizes or alignments
-	   in _bfd_set_section_content.  */
-      abfd->output_has_begun = TRUE;
-      break;
     }
 
   /* Record a copy of the data in memory if desired.  */

[signature.asc (application/pgp-signature, attachment)]

Message #31 received at 308624-close@bugs.debian.org (full text, mbox, reply):

From: Daniel Jacobowitz <dan@debian.org>

To: 308624-close@bugs.debian.org

Subject: Bug#308624: fixed in gdb 6.3-6

Date: Mon, 30 May 2005 09:17:15 -0400

Source: gdb
Source-Version: 6.3-6

We believe that the bug you reported is fixed in the latest version of
gdb, which is due to be installed in the Debian FTP archive:

gdb_6.3-6.diff.gz
  to pool/main/g/gdb/gdb_6.3-6.diff.gz
gdb_6.3-6.dsc
  to pool/main/g/gdb/gdb_6.3-6.dsc
gdb_6.3-6_i386.deb
  to pool/main/g/gdb/gdb_6.3-6_i386.deb



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 308624@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Daniel Jacobowitz <dan@debian.org> (supplier of updated gdb package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Sun, 29 May 2005 18:55:44 -0400
Source: gdb
Binary: gdb
Architecture: source i386
Version: 6.3-6
Distribution: unstable
Urgency: high
Maintainer: Daniel Jacobowitz <dan@debian.org>
Changed-By: Daniel Jacobowitz <dan@debian.org>
Description: 
 gdb        - The GNU Debugger
Closes: 308624 310728 310972
Changes: 
 gdb (6.3-6) unstable; urgency=high
 .
   * Do not crash on certain malformed input files [CAN-2005-1704]
     (Closes: #308624, #310972).
   * Do not load untrusted .gdbinit files [CAN-2005-1705] (Closes: #310728).
Files: 
 f991d2882842782116abd2cba9aec7bc 845 devel standard gdb_6.3-6.dsc
 5537c9d1ce95293a921ff4b5a55f5e5e 201861 devel standard gdb_6.3-6.diff.gz
 ec8ffd4489eca48d19ebb93e704a735b 2767820 devel standard gdb_6.3-6_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCmxC5bgOPXuCjg3cRAvW2AKCV7aSEbPZPuoZOJCbOgrZdaRpMCACgzaDN
pcJvKD4GUuWIFS7kSk1RTi0=
=8R1B
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.