This is an archive of the discontinued LLVM Phabricator instance.

[wasm] readSection: Avoid reading past eof (fixes oss-fuzz #3219)
ClosedPublic

Authored by vsk on Oct 10 2017, 6:58 PM.

Details

Summary

A wasm file crafted with a bogus section size can trigger an ASan issue
in the DWARFObjInMemory constructor. Nip the problem in the bud when we
read the wasm section.

Found by OSS-Fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3219

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Oct 10 2017, 6:58 PM
JDevlieghere added inline comments.Oct 13 2017, 3:00 PM
lib/DebugInfo/DWARF/DWARFContext.cpp
1159 ↗(On Diff #118524)

Should this be continue?

vsk added inline comments.Oct 13 2017, 3:02 PM
lib/DebugInfo/DWARF/DWARFContext.cpp
1159 ↗(On Diff #118524)

Oops! Yes. More to the point, the test doesn't cover this code.. I was wondering if you know how to test different error policies?

vsk added inline comments.Oct 13 2017, 3:06 PM
lib/DebugInfo/DWARF/DWARFContext.cpp
1159 ↗(On Diff #118524)

Hm, maybe I should just leave this change out.

JDevlieghere added inline comments.Oct 13 2017, 3:15 PM
lib/DebugInfo/DWARF/DWARFContext.cpp
1159 ↗(On Diff #118524)

I don't think llvm-dwarfdump implements this yet: it just calls DWARFContext::create with the defaultErrorHandler. Could you add an option to specify the policy?

sbc100 added inline comments.Oct 18 2017, 12:36 PM
lib/Object/WasmObjectFile.cpp
190 ↗(On Diff #118524)

Perhaps do this before the ArrayRef is created and before Ptr is incremented?

if (Ptr + Size > Eof)

test/tools/llvm-dwarfdump/X86/fuzzer.test
2 ↗(On Diff #118524)

Any reason this lives in 'X86'?

vsk updated this revision to Diff 119546.Oct 18 2017, 10:32 PM
vsk marked an inline comment as done.
vsk edited the summary of this revision. (Show Details)
  • Address review feedback, and set aside the dwarfdump changes for later.
test/tools/llvm-dwarfdump/X86/fuzzer.test
2 ↗(On Diff #118524)

I think it might be a mistake to conflate the libObject fix here with dwarfdump changes. I'll move the test to test/tools/llvm-objdump.

JDevlieghere accepted this revision.Oct 20 2017, 3:52 PM
This revision is now accepted and ready to land.Oct 20 2017, 3:52 PM
sbc100 accepted this revision.Oct 20 2017, 4:24 PM
This revision was automatically updated to reflect the committed changes.