[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

Should this be continue?

[vsk]

lib/DebugInfo/DWARF/DWARFContext.cpp

1159

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

Hm, maybe I should just leave this change out.

[JDevlieghere]

lib/DebugInfo/DWARF/DWARFContext.cpp

1159

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

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

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

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.

lib/DebugInfo/DWARF/DWARFContext.cpp

DWARFObjInMemory(const object::ObjectFile &Obj, const LoadedObjectInfo *L,
                    function_ref<ErrorPolicy(Error)> HandleError)
       : IsLittleEndian(Obj.isLittleEndian()),
         AddressSize(Obj.getBytesInAddress()), FileName(Obj.getFileName()),
         Obj(&Obj) {
 
     StringMap<unsigned> SectionAmountMap;
     for (const SectionRef &Section : Obj.sections()) {
       StringRef Name;
-      Section.getName(Name);
+      auto Err = Section.getName(Name);
+      if (Err) {
+        ErrorPolicy EP = HandleError(
+            createError("could not get section name in " + Obj.getFileName(),
+                        errorCodeToError(Err)));
+        if (EP == ErrorPolicy::Halt)
+          return;
+        return;
+      }
+
       ++SectionAmountMap[Name];
       SectionNames.push_back({ Name, true });
 
       // Skip BSS and Virtual sections, they aren't interesting.
       if (Section.isBSS() || Section.isVirtual())
         continue;
 
       // Skip sections stripped by dsymutil.

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);
   Section.Content = ArrayRef<uint8_t>(Ptr, Size);
   Ptr += Size;
+  if (Ptr > Eof)
+    return make_error<StringError>("Section too large",
+                                   object_error::parse_failed);
   return Error::success();
 }
 
 WasmObjectFile::WasmObjectFile(MemoryBufferRef Buffer, Error &Err)
     : ObjectFile(Binary::ID_Wasm, Buffer) {
   LinkingData.DataSize = 0;
 
   ErrorAsOutParameter ErrAsOutParam(&Err);
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);
   }
 }
 

test/tools/llvm-dwarfdump/X86/fuzzer.test

+RUN: not llvm-dwarfdump %S/Inputs/oss-fuzz-3219 2>&1 | FileCheck --check-prefix=FUZZ3219 %s
+FUZZ3219: oss-fuzz-3219: Invalid data was encountered while parsing the file