[BitcodeReader] Validate Strtab before accessing.

[fhahn]

Summary

This fixes a crash with invalid bitcode files that have records
referencing names in Strtab, but Strtab is not present or the index is
out-of-bounds.

This fixes the following clusterfuzz issue:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29895

[arsenm]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

3410–3415

I think you missed the word sure

[fhahn]

Add missing sure and slightly re-word comment, thanks!

[pcc]

I think there are more of these, search for Strtab.data() in that file.

[fhahn]

rebased, I'm planning on landing this soon.

In D95554#2531446, @pcc wrote:

I think there are more of these, search for Strtab.data() in that file.

Yes, those probably should also be audited. But I am not sure how to best get test cases for those. I'll take a look at some other oss-fuzz bugs.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
 
   if (Record.size() > 15) {
     Func->setDSOLocal(getDecodedDSOLocal(Record[15]));
   }
   inferDSOLocal(Func);
 
   // Record[16] is the address space number.
 
-  // Check whether we have enough values to read a partition name.
-  if (Record.size() > 18)
+  // Check whether we have enough values to read a partition name. Also make
+  // sure Strtab has enough values.
+  if (Record.size() > 18 && Strtab.data() &&
+      Record[17] + Record[18] <= Strtab.size()) {
     Func->setPartition(StringRef(Strtab.data() + Record[17], Record[18]));
+  }
 
   ValueList.push_back(Func);
 
   // If this is a function with a body, remember the prototype we are
   // creating now, so that we can match up the body with them later.
   if (!isProto) {
     Func->setIsMaterializable(true);
     FunctionsWithBodies.push_back(Func);

llvm/test/Bitcode/invalid-record-strtab.ll

+; Bitcode with an invalid record that indexes a name outside of strtab.
+
+; RUN: not llvm-dis %s.bc -o - 2>&1 | FileCheck %s
+
+; CHECK: error: Invalid record