Memory leaks and ineffective bounds checking in wasm_scan
authorAlan Modra <amodra@gmail.com>
Sun, 12 Jan 2020 21:42:18 +0000 (08:12 +1030)
committerAlan Modra <amodra@gmail.com>
Mon, 13 Jan 2020 01:42:05 +0000 (12:12 +1030)
commit0c0adcc52478ebb707ed780173e18262df6eab7e
treeca374cabe55e1317f9eb1e0c07d92d8f2f66f771
parent5496abe1c5c31aa6648e8fdb15e4122025bcabfe
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.
bfd/ChangeLog
bfd/wasm-module.c
This page took 0.025027 seconds and 4 git commands to generate.