Fixes for invalid memory accesses triggered by running windres on corrupt binaries.

PR binutils/17512
	* rcparse.y: Add checks to avoid integer divide by zero.
	* rescoff.c (read_coff_rsrc): Add check on the size of the
	resource section.
	(read_coff_res_dir): Add check on the nesting level.
	Check for resource names overrunning the buffer.
	* resrc.c (write_rc_messagetable): Update formatting.
	Add check of 'elen' being zero.
This commit is contained in:
Nick Clifton 2015-01-27 17:32:23 +00:00
parent 877a8638ba
commit 0897ec1581
4 changed files with 89 additions and 52 deletions

View file

@ -7,6 +7,14 @@
* addr2line.c (slurp_symtab): If the symcount is zero, free the * addr2line.c (slurp_symtab): If the symcount is zero, free the
symbol table pointer. symbol table pointer.
* rcparse.y: Add checks to avoid integer divide by zero.
* rescoff.c (read_coff_rsrc): Add check on the size of the
resource section.
(read_coff_res_dir): Add check on the nesting level.
Check for resource names overrunning the buffer.
* resrc.c (write_rc_messagetable): Update formatting.
Add check of 'elen' being zero.
2015-01-23 Nick Clifton <nickc@redhat.com> 2015-01-23 Nick Clifton <nickc@redhat.com>
* nlmconv.c (powerpc_mangle_relocs): Fix build errors introduced * nlmconv.c (powerpc_mangle_relocs): Fix build errors introduced

View file

@ -1887,12 +1887,12 @@ sizednumexpr:
} }
| sizednumexpr '/' sizednumexpr | sizednumexpr '/' sizednumexpr
{ {
$$.val = $1.val / $3.val; $$.val = $1.val / ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword; $$.dword = $1.dword || $3.dword;
} }
| sizednumexpr '%' sizednumexpr | sizednumexpr '%' sizednumexpr
{ {
$$.val = $1.val % $3.val; $$.val = $1.val % ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword; $$.dword = $1.dword || $3.dword;
} }
| sizednumexpr '+' sizednumexpr | sizednumexpr '+' sizednumexpr
@ -1966,12 +1966,13 @@ sizedposnumexpr:
} }
| sizedposnumexpr '/' sizednumexpr | sizedposnumexpr '/' sizednumexpr
{ {
$$.val = $1.val / $3.val; $$.val = $1.val / ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword; $$.dword = $1.dword || $3.dword;
} }
| sizedposnumexpr '%' sizednumexpr | sizedposnumexpr '%' sizednumexpr
{ {
$$.val = $1.val % $3.val; /* PR 17512: file: 89105a25. */
$$.val = $1.val % ($3.val ? $3.val : 1);
$$.dword = $1.dword || $3.dword; $$.dword = $1.dword || $3.dword;
} }
| sizedposnumexpr '+' sizednumexpr | sizedposnumexpr '+' sizednumexpr

View file

@ -142,8 +142,14 @@ read_coff_rsrc (const char *filename, const char *target)
set_windres_bfd (&wrbfd, abfd, sec, WR_KIND_BFD); set_windres_bfd (&wrbfd, abfd, sec, WR_KIND_BFD);
size = bfd_section_size (abfd, sec); size = bfd_section_size (abfd, sec);
data = (bfd_byte *) res_alloc (size); /* PR 17512: file: 1b25ba5d
The call to get_file_size here may be expensive
but there is no other way to determine if the section size
is reasonable. */
if (size > (bfd_size_type) get_file_size (filename))
fatal (_("%s: .rsrc section is bigger than the file!"), filename);
data = (bfd_byte *) res_alloc (size);
get_windres_bfd_content (&wrbfd, data, 0, size); get_windres_bfd_content (&wrbfd, data, 0, size);
flaginfo.filename = filename; flaginfo.filename = filename;
@ -185,6 +191,13 @@ read_coff_res_dir (windres_bfd *wrbfd, const bfd_byte *data,
rc_res_entry **pp; rc_res_entry **pp;
const struct extern_res_entry *ere; const struct extern_res_entry *ere;
/* PR 17512: file: 09d80f53.
Whilst in theory resources can nest to any level, in practice
Microsoft only defines 3 levels. Corrupt files however might
claim to use more. */
if (level > 4)
overrun (flaginfo, _("Resources nest too deep"));
if ((size_t) (flaginfo->data_end - data) < sizeof (struct extern_res_directory)) if ((size_t) (flaginfo->data_end - data) < sizeof (struct extern_res_directory))
overrun (flaginfo, _("directory")); overrun (flaginfo, _("directory"));
@ -234,7 +247,12 @@ read_coff_res_dir (windres_bfd *wrbfd, const bfd_byte *data,
re->id.u.n.length = length; re->id.u.n.length = length;
re->id.u.n.name = (unichar *) res_alloc (length * sizeof (unichar)); re->id.u.n.name = (unichar *) res_alloc (length * sizeof (unichar));
for (j = 0; j < length; j++) for (j = 0; j < length; j++)
{
/* PR 17512: file: 05dc4a16. */
if (length < 0 || ers >= (bfd_byte *) ere || ers + j * 2 + 4 >= (bfd_byte *) ere)
overrun (flaginfo, _("resource name"));
re->id.u.n.name[j] = windres_get_16 (wrbfd, ers + j * 2 + 2, 2); re->id.u.n.name[j] = windres_get_16 (wrbfd, ers + j * 2 + 2, 2);
}
if (level == 0) if (level == 0)
type = &re->id; type = &re->id;

View file

@ -2932,10 +2932,13 @@ write_rc_messagetable (FILE *e, rc_uint_type length, const bfd_byte *data)
if (length < BIN_MESSAGETABLE_SIZE) if (length < BIN_MESSAGETABLE_SIZE)
has_error = 1; has_error = 1;
else else
do { do
{
rc_uint_type m, i; rc_uint_type m, i;
mt = (const struct bin_messagetable *) data; mt = (const struct bin_messagetable *) data;
m = windres_get_32 (&wrtarget, mt->cblocks, length); m = windres_get_32 (&wrtarget, mt->cblocks, length);
if (length < (BIN_MESSAGETABLE_SIZE + m * BIN_MESSAGETABLE_BLOCK_SIZE)) if (length < (BIN_MESSAGETABLE_SIZE + m * BIN_MESSAGETABLE_BLOCK_SIZE))
{ {
has_error = 1; has_error = 1;
@ -2967,18 +2970,25 @@ write_rc_messagetable (FILE *e, rc_uint_type length, const bfd_byte *data)
} }
wr_printcomment (e, "MessageId = 0x%x", low); wr_printcomment (e, "MessageId = 0x%x", low);
wr_printcomment (e, ""); wr_printcomment (e, "");
/* PR 17512: file: 5c3232dc. */
if (elen)
{
if ((flags & MESSAGE_RESOURCE_UNICODE) == MESSAGE_RESOURCE_UNICODE) if ((flags & MESSAGE_RESOURCE_UNICODE) == MESSAGE_RESOURCE_UNICODE)
unicode_print (e, (const unichar *) mti->data, unicode_print (e, (const unichar *) mti->data,
(elen - BIN_MESSAGETABLE_ITEM_SIZE) / 2); (elen - BIN_MESSAGETABLE_ITEM_SIZE) / 2);
else else
ascii_print (e, (const char *) mti->data, ascii_print (e, (const char *) mti->data,
(elen - BIN_MESSAGETABLE_ITEM_SIZE)); (elen - BIN_MESSAGETABLE_ITEM_SIZE));
}
wr_printcomment (e,""); wr_printcomment (e,"");
++low; ++low;
offset += elen; offset += elen;
} }
} }
} while (0); }
while (0);
if (has_error) if (has_error)
wr_printcomment (e, "Illegal data"); wr_printcomment (e, "Illegal data");
wr_print_flush (e); wr_print_flush (e);