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
===================================================================
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
Description: OpenPGP digital signature
