Memory leaks and ineffective bounds checking in wasm_scan
It's always a bad idea to perform arithmetic on an unknown value read from an object file before comparing against bounds. Code like the following attempting to bounds check "len", a 64-bit value, isn't effective because the pointer arithmetic ignores the high 32 bits when compiled for a 32-bit host. READ_LEB128 (len, p, end); if (p + len < p || p + len > end) goto error_return; Instead, perform any arithmetic on known values where we don't need to worry about overflows: READ_LEB128 (len, p, end); if (len > (size_t) (end - p)) goto error_return; I'll note that this check does do things the right way: READ_LEB128 (symcount, p, end); /* Sanity check: each symbol has at least two bytes. */ if (symcount > payload_size / 2) return FALSE; "symcount * 2 > payload_size" would be wrong since the multiply could overflow. * wasm-module.c (wasm_scan_name_function_section): Formatting. Delete asect name check. Move asect NULL check to wasm_object_p. Correct bounds check of sizes against end. Replace uses of bfd_zalloc with bfd_alloc, zeroing only necessary bytes. Use just one bfd_release. (wasm_scan): Don't use malloc/strdup for section names, bfd_alloc instead. Simplify code prefixing section name. Formatting. Don't attempt to free memory here.. (wasm_object_p): ..do so here. Formatting.
This commit is contained in:
parent
5496abe1c5
commit
0c0adcc524
|
@ -1,3 +1,15 @@
|
|||
2020-01-13 Alan Modra <amodra@gmail.com>
|
||||
|
||||
* wasm-module.c (wasm_scan_name_function_section): Formatting.
|
||||
Delete asect name check. Move asect NULL check to wasm_object_p.
|
||||
Correct bounds check of sizes against end. Replace uses of
|
||||
bfd_zalloc with bfd_alloc, zeroing only necessary bytes. Use
|
||||
just one bfd_release.
|
||||
(wasm_scan): Don't use malloc/strdup for section names,
|
||||
bfd_alloc instead. Simplify code prefixing section name.
|
||||
Formatting. Don't attempt to free memory here..
|
||||
(wasm_object_p): ..do so here.
|
||||
|
||||
2020-01-10 Szabolcs Nagy <szabolcs.nagy@arm.com>
|
||||
|
||||
PR ld/22269
|
||||
|
|
|
@ -246,16 +246,10 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
asymbol *symbols = NULL;
|
||||
sec_ptr space_function_index;
|
||||
|
||||
if (! asect)
|
||||
return FALSE;
|
||||
|
||||
if (strcmp (asect->name, WASM_NAME_SECTION) != 0)
|
||||
return FALSE;
|
||||
|
||||
p = asect->contents;
|
||||
end = asect->contents + asect->size;
|
||||
|
||||
if (! p)
|
||||
if (!p)
|
||||
return FALSE;
|
||||
|
||||
while (p < end)
|
||||
|
@ -272,7 +266,7 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
|
||||
READ_LEB128 (payload_size, p, end);
|
||||
|
||||
if (p > p + payload_size)
|
||||
if (payload_size > (size_t) (end - p))
|
||||
return FALSE;
|
||||
|
||||
p += payload_size;
|
||||
|
@ -283,10 +277,7 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
|
||||
READ_LEB128 (payload_size, p, end);
|
||||
|
||||
if (p > p + payload_size)
|
||||
return FALSE;
|
||||
|
||||
if (p + payload_size > end)
|
||||
if (payload_size > (size_t) (end - p))
|
||||
return FALSE;
|
||||
|
||||
end = p + payload_size;
|
||||
|
@ -294,22 +285,24 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
READ_LEB128 (symcount, p, end);
|
||||
|
||||
/* Sanity check: each symbol has at least two bytes. */
|
||||
if (symcount > payload_size/2)
|
||||
if (symcount > payload_size / 2)
|
||||
return FALSE;
|
||||
|
||||
tdata->symcount = symcount;
|
||||
|
||||
space_function_index = bfd_make_section_with_flags
|
||||
(abfd, WASM_SECTION_FUNCTION_INDEX, SEC_READONLY | SEC_CODE);
|
||||
space_function_index
|
||||
= bfd_make_section_with_flags (abfd, WASM_SECTION_FUNCTION_INDEX,
|
||||
SEC_READONLY | SEC_CODE);
|
||||
|
||||
if (! space_function_index)
|
||||
space_function_index = bfd_get_section_by_name (abfd, WASM_SECTION_FUNCTION_INDEX);
|
||||
if (!space_function_index)
|
||||
space_function_index
|
||||
= bfd_get_section_by_name (abfd, WASM_SECTION_FUNCTION_INDEX);
|
||||
|
||||
if (! space_function_index)
|
||||
if (!space_function_index)
|
||||
return FALSE;
|
||||
|
||||
symbols = bfd_zalloc (abfd, tdata->symcount * sizeof (asymbol));
|
||||
if (! symbols)
|
||||
symbols = bfd_alloc2 (abfd, tdata->symcount, sizeof (asymbol));
|
||||
if (!symbols)
|
||||
return FALSE;
|
||||
|
||||
for (symcount = 0; p < end && symcount < tdata->symcount; symcount++)
|
||||
|
@ -322,14 +315,15 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
READ_LEB128 (idx, p, end);
|
||||
READ_LEB128 (len, p, end);
|
||||
|
||||
if (p + len < p || p + len > end)
|
||||
if (len > (size_t) (end - p))
|
||||
goto error_return;
|
||||
|
||||
name = bfd_zalloc (abfd, len + 1);
|
||||
if (! name)
|
||||
name = bfd_alloc (abfd, len + 1);
|
||||
if (!name)
|
||||
goto error_return;
|
||||
|
||||
memcpy (name, p, len);
|
||||
name[len] = 0;
|
||||
p += len;
|
||||
|
||||
sym = &symbols[symcount];
|
||||
|
@ -350,8 +344,6 @@ wasm_scan_name_function_section (bfd *abfd, sec_ptr asect)
|
|||
return TRUE;
|
||||
|
||||
error_return:
|
||||
while (symcount)
|
||||
bfd_release (abfd, (void *)symbols[--symcount].name);
|
||||
bfd_release (abfd, symbols);
|
||||
return FALSE;
|
||||
}
|
||||
|
@ -386,13 +378,12 @@ wasm_scan (bfd *abfd)
|
|||
bfd_vma vma = 0x80000000;
|
||||
int section_code;
|
||||
unsigned int bytes_read;
|
||||
char *name = NULL;
|
||||
asection *bfdsec;
|
||||
|
||||
if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)
|
||||
goto error_return;
|
||||
|
||||
if (! wasm_read_header (abfd, &error))
|
||||
if (!wasm_read_header (abfd, &error))
|
||||
goto error_return;
|
||||
|
||||
while ((section_code = wasm_read_byte (abfd, &error)) != EOF)
|
||||
|
@ -401,14 +392,13 @@ wasm_scan (bfd *abfd)
|
|||
{
|
||||
const char *sname = wasm_section_code_to_name (section_code);
|
||||
|
||||
if (! sname)
|
||||
if (!sname)
|
||||
goto error_return;
|
||||
|
||||
name = strdup (sname);
|
||||
bfdsec = bfd_make_section_anyway_with_flags (abfd, name, SEC_HAS_CONTENTS);
|
||||
bfdsec = bfd_make_section_anyway_with_flags (abfd, sname,
|
||||
SEC_HAS_CONTENTS);
|
||||
if (bfdsec == NULL)
|
||||
goto error_return;
|
||||
name = NULL;
|
||||
|
||||
bfdsec->vma = vma;
|
||||
bfdsec->lma = vma;
|
||||
|
@ -423,9 +413,9 @@ wasm_scan (bfd *abfd)
|
|||
bfd_vma payload_len;
|
||||
file_ptr section_start;
|
||||
bfd_vma namelen;
|
||||
char *name;
|
||||
char *prefix = WASM_SECTION_PREFIX;
|
||||
char *p;
|
||||
int ret;
|
||||
size_t prefixlen = strlen (prefix);
|
||||
|
||||
payload_len = wasm_read_leb128 (abfd, &error, &bytes_read, FALSE);
|
||||
if (error)
|
||||
|
@ -434,21 +424,18 @@ wasm_scan (bfd *abfd)
|
|||
namelen = wasm_read_leb128 (abfd, &error, &bytes_read, FALSE);
|
||||
if (error || namelen > payload_len)
|
||||
goto error_return;
|
||||
name = bfd_zmalloc (namelen + strlen (prefix) + 1);
|
||||
if (! name)
|
||||
name = bfd_alloc (abfd, namelen + prefixlen + 1);
|
||||
if (!name)
|
||||
goto error_return;
|
||||
p = name;
|
||||
ret = sprintf (p, "%s", prefix);
|
||||
if (ret < 0 || (bfd_vma) ret != strlen (prefix))
|
||||
goto error_return;
|
||||
p += ret;
|
||||
if (bfd_bread (p, namelen, abfd) != namelen)
|
||||
memcpy (name, prefix, prefixlen);
|
||||
if (bfd_bread (name + prefixlen, namelen, abfd) != namelen)
|
||||
goto error_return;
|
||||
name[prefixlen + namelen] = 0;
|
||||
|
||||
bfdsec = bfd_make_section_anyway_with_flags (abfd, name, SEC_HAS_CONTENTS);
|
||||
bfdsec = bfd_make_section_anyway_with_flags (abfd, name,
|
||||
SEC_HAS_CONTENTS);
|
||||
if (bfdsec == NULL)
|
||||
goto error_return;
|
||||
name = NULL;
|
||||
|
||||
bfdsec->vma = vma;
|
||||
bfdsec->lma = vma;
|
||||
|
@ -459,8 +446,8 @@ wasm_scan (bfd *abfd)
|
|||
|
||||
if (bfdsec->size != 0)
|
||||
{
|
||||
bfdsec->contents = bfd_zalloc (abfd, bfdsec->size);
|
||||
if (! bfdsec->contents)
|
||||
bfdsec->contents = bfd_alloc (abfd, bfdsec->size);
|
||||
if (!bfdsec->contents)
|
||||
goto error_return;
|
||||
|
||||
if (bfd_bread (bfdsec->contents, bfdsec->size, abfd) != bfdsec->size)
|
||||
|
@ -478,12 +465,6 @@ wasm_scan (bfd *abfd)
|
|||
return TRUE;
|
||||
|
||||
error_return:
|
||||
if (name)
|
||||
free (name);
|
||||
|
||||
for (bfdsec = abfd->sections; bfdsec; bfdsec = bfdsec->next)
|
||||
free ((void *) bfdsec->name);
|
||||
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
|
@ -750,23 +731,30 @@ static const bfd_target *
|
|||
wasm_object_p (bfd *abfd)
|
||||
{
|
||||
bfd_boolean error;
|
||||
asection *s;
|
||||
|
||||
if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)
|
||||
return NULL;
|
||||
|
||||
if (! wasm_read_header (abfd, &error))
|
||||
if (!wasm_read_header (abfd, &error))
|
||||
{
|
||||
bfd_set_error (bfd_error_wrong_format);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (! wasm_mkobject (abfd) || ! wasm_scan (abfd))
|
||||
if (!wasm_mkobject (abfd))
|
||||
return NULL;
|
||||
|
||||
if (! bfd_default_set_arch_mach (abfd, bfd_arch_wasm32, 0))
|
||||
return NULL;
|
||||
if (!wasm_scan (abfd)
|
||||
|| !bfd_default_set_arch_mach (abfd, bfd_arch_wasm32, 0))
|
||||
{
|
||||
bfd_release (abfd, abfd->tdata.any);
|
||||
abfd->tdata.any = NULL;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (wasm_scan_name_function_section (abfd, bfd_get_section_by_name (abfd, WASM_NAME_SECTION)))
|
||||
s = bfd_get_section_by_name (abfd, WASM_NAME_SECTION);
|
||||
if (s != NULL && wasm_scan_name_function_section (abfd, s))
|
||||
abfd->flags |= HAS_SYMS;
|
||||
|
||||
return abfd->xvec;
|
||||
|
|
Loading…
Reference in New Issue