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

[vsk]

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

[JDevlieghere]

lib/DebugInfo/DWARF/DWARFContext.cpp

1159 ↗

(On Diff #118524)

Should this be continue?

[vsk]

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]

lib/DebugInfo/DWARF/DWARFContext.cpp

1159 ↗

(On Diff #118524)

Hm, maybe I should just leave this change out.

[JDevlieghere]

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]

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]

• 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.

llvm/trunk/lib/Object/WasmObjectFile.cpp

 static wasm::WasmTable readTable(const uint8_t *&Ptr) {
   wasm::WasmTable Table;
   Table.ElemType = readVarint7(Ptr);
   Table.Limits = readLimits(Ptr);
   return Table;
 }
 
 static Error readSection(WasmSection &Section, const uint8_t *&Ptr,
-                         const uint8_t *Start) {
-  // TODO(sbc): Avoid reading past EOF in the case of malformed files.
+                         const uint8_t *Start, const uint8_t *Eof) {
   Section.Offset = Ptr - Start;
   Section.Type = readVaruint7(Ptr);
   uint32_t Size = readVaruint32(Ptr);
   if (Size == 0)
     return make_error<StringError>("Zero length section",
                                    object_error::parse_failed);
+  if (Ptr + Size > Eof)
+    return make_error<StringError>("Section too large",
+                                   object_error::parse_failed);
   Section.Content = ArrayRef<uint8_t>(Ptr, Size);
   Ptr += Size;
   return Error::success();
 }
 
 WasmObjectFile::WasmObjectFile(MemoryBufferRef Buffer, Error &Err)
     : ObjectFile(Binary::ID_Wasm, Buffer) {
   LinkingData.DataSize = 0;
WasmObjectFile::WasmObjectFile(MemoryBufferRef Buffer, Error &Err)
   if (Header.Version != wasm::WasmVersion) {
     Err = make_error<StringError>("Bad version number",
                                   object_error::parse_failed);
     return;
   }
 
   WasmSection Sec;
   while (Ptr < Eof) {
-    if ((Err = readSection(Sec, Ptr, getPtr(0))))
+    if ((Err = readSection(Sec, Ptr, getPtr(0), Eof)))
       return;
     if ((Err = parseSection(Sec)))
       return;
 
     Sections.push_back(Sec);
   }
 }
 

llvm/trunk/test/tools/llvm-objdump/wasm-corrupt-section.test

+# RUN: not llvm-objdump -h %p/Inputs/corrupt-section.wasm 2>&1 | FileCheck %s
+# CHECK: '{{.*}}corrupt-section.wasm': Section too large