bug: do not allow negative /Prev value

[eknoes]

The following issue exists, when /Prev is negative:

The /Prev value describes an offset from the start of a file. Thus, a negative /Prev value can lead to a heap buffer overflow (read) which is detected with -fsanitize=adress:

==3957970==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7ffff555f49a at pc 0x5555556e89ae bp 0x7fffffffc400 sp 0x7fffffffbbc0 READ of size 1 at 0x7ffff555f49a thread T0
[Detaching after fork from child process 3957971]
    #0 0x5555556e89ad in __asan_memcpy (PDF-Writer/build/PDFFuzzHarness+0x1949ad) (BuildId: e0c027e66daf6a9d)
    #1 0x555555737c25 in InputByteArrayStream::Read(unsigned char*, unsigned long) PDF-Writer/PDFWriter/InputByteArrayStream.cpp:58:3
    #2 0x5555557cf25b in PDFParserTokenizer::GetNextByteForToken(unsigned char&) PDF-Writer/PDFWriter/PDFParserTokenizer.cpp:376:20
    #3 0x5555557cee1c in PDFParserTokenizer::SkipTillToken() PDF-Writer/PDFWriter/PDFParserTokenizer.cpp:351:6
    #4 0x5555557cb33e in PDFParserTokenizer::GetNextToken[abi:cxx11]() PDF-Writer/PDFWriter/PDFParserTokenizer.cpp:79:3
    #5 0x5555557768dd in PDFObjectParser::GetNextToken(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) PDF-Writer/PDFWriter/PDFObjectParser.cpp:252:33
    #6 0x555555774d07 in PDFObjectParser::ParseNewObject() PDF-Writer/PDFWriter/PDFObjectParser.cpp:98:7
    #7 0x555555752eaa in PDFParser::ParsePreviousFileDirectory(long long, std::vector<XrefEntryInput, std::allocator<XrefEntryInput>>&, unsigned long, PDFDictionary**, unsigned long*) PDF-Writer/PDFWriter/PDFParser.cpp:1117:49
    #8 0x555555745b7b in PDFParser::ParsePreviousXrefs(PDFDictionary*) PDF-Writer/PDFWriter/PDFParser.cpp:1084:12
    #9 0x555555745db9 in PDFParser::ParsePreviousXrefs(PDFDictionary*) PDF-Writer/PDFWriter/PDFParser.cpp:1091:13
    #10 0x555555743e55 in PDFParser::BuildXrefTableFromTable() PDF-Writer/PDFWriter/PDFParser.cpp:500:13
    #11 0x55555573e90f in PDFParser::ParseFileDirectory() PDF-Writer/PDFWriter/PDFParser.cpp:1269:13
    #12 0x55555573b436 in PDFParser::StartPDFParsing(IByteReaderWithPosition*, PDFParsingOptions const&) PDF-Writer/PDFWriter/PDFParser.cpp:120:12

0x7ffff555f49a is located 870 bytes before 1048576-byte region [0x7ffff555f800,0x7ffff565f800) allocated by thread T0

[eknoes]

I think the real issue is in the methods of InputByteArrayStream: Currently, depending on the function, only the upper or lower bound is checked for reads, skips, .... I think always clamping memory acces to 0..mArrayLength makes the code more secure.

[galkahana]

cheers, thanks. and i like the clamping solution very much. i'll take this, and add later two mods:

• print the negative value as part of the trace
• reuse the sanity test, instead of a separate new test case in code

[galkahana]

hmm clamp is c++17. i'll have to use a manual method then.