Verify sizes when trying to read a BitcodeAbbrevOp

[filcab]

Summary

Make sure we don't try to read more than we can when reading abbrev
operands.

Bug found with AFL fuzz.

[• rafael]

include/llvm/Bitcode/BitstreamReader.h

215 ↗

(On Diff #23782)

getMaxChunkSize.

Or just make this a constant. It doesn't need to be a function, right?

test/Bitcode/invalid.test

59 ↗

(On Diff #23782)

If you have a testcase handy for the 4 cases, please add them.

[filcab]

True. I'll change it to be a constant.

[filcab]

Submitted before I wanted.
I'll also figure it out and add tests for each case.

[filcab]

Changed MaxChunkSize to be a constant.
Made the check earlier, when abbrevs are read, and added asserts to
read/skipAbbreviatedField.
Replaced the test with two tests, one for Fixed field, and one for a VBR
field.

[• rafael]

LGTM

llvm/trunk/include/llvm/Bitcode/BitstreamReader.h

struct Block {
     explicit Block(unsigned PCS) : PrevCodeSize(PCS) {}
   };
 
   /// This tracks the codesize of parent blocks.
   SmallVector<Block, 8> BlockScope;
 
 
 public:
+  static const size_t MaxChunkSize = sizeof(word_t) * 8;
+
   BitstreamCursor() { init(nullptr); }
 
   explicit BitstreamCursor(BitstreamReader &R) { init(&R); }
 
   void init(BitstreamReader *R) {
     freeState();
 
     BitStream = R;
void fillCurWord() {
     CurWord =
         support::endian::read<word_t, support::little, support::unaligned>(
             Array);
     NextChar += BytesRead;
     BitsInCurWord = BytesRead * 8;
   }
 
   word_t Read(unsigned NumBits) {
-    static const unsigned BitsInWord = sizeof(word_t) * 8;
+    static const unsigned BitsInWord = MaxChunkSize;
 
     assert(NumBits && NumBits <= BitsInWord &&
            "Cannot return zero or more than BitsInWord bits!");
 
     static const unsigned Mask = sizeof(word_t) > 4 ? 0x3f : 0x1f;
 
     // If the field is fully contained by CurWord, return it quickly.
     if (BitsInCurWord >= NumBits) {

llvm/trunk/lib/Bitcode/Reader/BitstreamReader.cpp

static uint64_t readAbbreviatedField(BitstreamCursor &Cursor,
   assert(!Op.isLiteral() && "Not to be used with literals!");
 
   // Decode the value as we are commanded.
   switch (Op.getEncoding()) {
   case BitCodeAbbrevOp::Array:
   case BitCodeAbbrevOp::Blob:
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
+    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     return Cursor.Read((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::VBR:
+    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     return Cursor.ReadVBR64((unsigned)Op.getEncodingData());
   case BitCodeAbbrevOp::Char6:
     return BitCodeAbbrevOp::DecodeChar6(Cursor.Read(6));
   }
   llvm_unreachable("invalid abbreviation encoding");
 }
 
 static void skipAbbreviatedField(BitstreamCursor &Cursor,
                                  const BitCodeAbbrevOp &Op) {
   assert(!Op.isLiteral() && "Not to be used with literals!");
 
   // Decode the value as we are commanded.
   switch (Op.getEncoding()) {
   case BitCodeAbbrevOp::Array:
   case BitCodeAbbrevOp::Blob:
     llvm_unreachable("Should not reach here");
   case BitCodeAbbrevOp::Fixed:
+    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     Cursor.Read((unsigned)Op.getEncodingData());
     break;
   case BitCodeAbbrevOp::VBR:
+    assert((unsigned)Op.getEncodingData() <= Cursor.MaxChunkSize);
     Cursor.ReadVBR64((unsigned)Op.getEncodingData());
     break;
   case BitCodeAbbrevOp::Char6:
     Cursor.Read(6);
     break;
   }
 }
 
if (BitCodeAbbrevOp::hasEncodingData(E)) {
       // and vbr(0) as a literal zero.  This is decoded the same way, and avoids
       // a slow path in Read() to have to handle reading zero bits.
       if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
           Data == 0) {
         Abbv->Add(BitCodeAbbrevOp(0));
         continue;
       }
 
+      if ((E == BitCodeAbbrevOp::Fixed || E == BitCodeAbbrevOp::VBR) &&
+          Data > MaxChunkSize)
+        report_fatal_error(
+            "Fixed or VBR abbrev record with size > MaxChunkData");
+
       Abbv->Add(BitCodeAbbrevOp(E, Data));
     } else
       Abbv->Add(BitCodeAbbrevOp(E));
   }
   CurAbbrevs.push_back(Abbv);
 }
 
 bool BitstreamCursor::ReadBlockInfoBlock() {

llvm/trunk/test/Bitcode/invalid.test

 RUN:   FileCheck --check-prefix=NO-MODULE %s
 
 NO-MODULE: Malformed IR file
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-fp-shift.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=FP-SHIFT %s
 
 FP-SHIFT: Invalid record
+
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev-vbr-size-too-big.bc 2>&1 | \
+RUN:   FileCheck --check-prefix=HUGE-ABBREV-OP %s
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-abbrev-fixed-size-too-big.bc 2>&1 | \
+RUN:   FileCheck --check-prefix=HUGE-ABBREV-OP %s
+
+HUGE-ABBREV-OP: Fixed or VBR abbrev record with size > MaxChunkData