From 2584e1e324c97eeeacc1e421e5f3191a708c3d2d Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Thu, 19 Nov 2020 12:18:18 +0300 Subject: [PATCH] [llvm-readobj] - Don't crash when relocation table goes past the EOF. It is possible to trigger reading past the EOF by breaking fields like DT_PLTRELSZ, DT_RELSZ or DT_RELASZ This patch adds a validation in `DynRegionInfo` helper class. Differential revision: https://reviews.llvm.org/D91787 --- .../llvm-readobj/ELF/broken-dynamic-reloc.test | 79 ++++++++++++++++++---- llvm/tools/llvm-readobj/ELFDumper.cpp | 43 ++++++++---- 2 files changed, 92 insertions(+), 30 deletions(-) diff --git a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test index bd7916a9..741eaec 100644 --- a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test +++ b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test @@ -49,11 +49,16 @@ ProgramHeaders: LastSec: .dynamic ## Show we print a warning for an invalid relocation table size stored in a DT_RELASZ entry. -# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0xFF -DTAG2=DT_RELAENT %s -o %t2 -# RUN: llvm-readobj --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ -# RUN: llvm-readelf --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ -# INVALID-DT-RELASZ: warning: '[[FILE]]': invalid DT_RELASZ value (0xff) or DT_RELAENT value (0x18) +## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning + +# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0xFF -DTAG2=DT_RELAENT %s -o %t2a +# RUN: llvm-readobj --dyn-relocations %t2a 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2a --check-prefix=INVALID-DT-RELASZ1 --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-relocations %t2a 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2a --check-prefix=INVALID-DT-RELASZ1 --implicit-check-not=warning: + +# INVALID-DT-RELASZ1: warning: '[[FILE]]': invalid DT_RELASZ value (0xff) or DT_RELAENT value (0x18) --- !ELF FileHeader: @@ -80,19 +85,42 @@ ProgramHeaders: FirstSec: .relx.dyn LastSec: .dynamic +## Case B: the DT_RELASZ has value of 0x251, what is too large, because the relocation table goes past the EOF. + +# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0x251 -DTAG2=DT_RELAENT %s -o %t2b +# RUN: llvm-readobj --dyn-relocations %t2b 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2b --check-prefix=INVALID-DT-RELASZ2 --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-relocations %t2b 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2b --check-prefix=INVALID-DT-RELASZ2 --implicit-check-not=warning: + +# INVALID-DT-RELASZ2: warning: '[[FILE]]': unable to read data at 0x78 of size 0x251 (DT_RELASZ value): it goes past the end of the file of size 0x2c8 + ## Show we print a warning for an invalid relocation table entry size stored in a DT_RELAENT entry. # RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG2=DT_RELAENT -DTAG2VAL=0xFF %s -o %t3 -# RUN: llvm-readobj --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT -# RUN: llvm-readelf --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT +# RUN: llvm-readobj --dyn-relocations %t3 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-relocations %t3 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT --implicit-check-not=warning: ## INVALID-DT-RELAENT: warning: '[[FILE]]': invalid DT_RELASZ value (0x18) or DT_RELAENT value (0xff) ## Show we print a warning for an invalid relocation table size stored in a DT_RELSZ entry. -# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0xFF -DTAG2=DT_RELENT %s -o %t4 -# RUN: llvm-readobj --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ -# RUN: llvm-readelf --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ -## INVALID-DT-RELSZ: warning: '[[FILE]]': invalid DT_RELSZ value (0xff) or DT_RELENT value (0x18) +## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning. + +# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0xFF -DTAG2=DT_RELENT %s -o %t4a +# RUN: llvm-readobj --dyn-relocations %t4a 2>&1 | FileCheck %s -DFILE=%t4a --check-prefix=INVALID-DT-RELSZ1 +# RUN: llvm-readelf --dyn-relocations %t4a 2>&1 | FileCheck %s -DFILE=%t4a --check-prefix=INVALID-DT-RELSZ1 + +## INVALID-DT-RELSZ1: warning: '[[FILE]]': invalid DT_RELSZ value (0xff) or DT_RELENT value (0x18) + +## Case B: the DT_RELSZ has value of 0x251, what is too large, because the relocation table goes past the EOF. + +# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0x251 -DTAG2=DT_RELENT %s -o %t4b +# RUN: llvm-readobj --dyn-relocations %t4b 2>&1 | FileCheck %s -DFILE=%t4b --check-prefix=INVALID-DT-RELSZ2 +# RUN: llvm-readelf --dyn-relocations %t4b 2>&1 | FileCheck %s -DFILE=%t4b --check-prefix=INVALID-DT-RELSZ2 + +# INVALID-DT-RELSZ2: warning: '[[FILE]]': unable to read data at 0x78 of size 0x251 (DT_RELSZ value): it goes past the end of the file of size 0x2c8 ## Show we print a warning for an invalid relocation table entry size stored in a DT_RELENT entry. # RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG2=DT_RELENT -DTAG2VAL=0xFF %s -o %t5 @@ -126,11 +154,16 @@ ProgramHeaders: ## Show we print a warning for an invalid value of DT_PLTRELSZ, which describes the total size ## of the relocation entries associated with the procedure linkage table. -# RUN: yaml2obj --docnum=3 %s -o %t10 -# RUN: llvm-readobj --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ -# RUN: llvm-readelf --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ -# INVALID-DT-PLTRELSZ: warning: '[[FILE]]': invalid DT_PLTRELSZ value (0xff){{$}} +## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning. + +# RUN: yaml2obj --docnum=3 -DVAL=0xFF %s -o %t10a +# RUN: llvm-readobj --dyn-relocations %t10a 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t10a --check-prefix=INVALID-DT-PLTRELSZ1 --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-relocations %t10a 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t10a --check-prefix=INVALID-DT-PLTRELSZ1 --implicit-check-not=warning: + +# INVALID-DT-PLTRELSZ1: warning: '[[FILE]]': invalid DT_PLTRELSZ value (0xff){{$}} --- !ELF FileHeader: @@ -149,7 +182,7 @@ Sections: - Tag: DT_JMPREL Value: 0x0 - Tag: DT_PLTRELSZ - Value: 0xFF ## The valid value would be 0x18. + Value: [[VAL]] ## The valid value would be 0x18. - Tag: DT_PLTREL Value: 0x7 ## DT_RELA - Tag: DT_NULL @@ -160,6 +193,22 @@ ProgramHeaders: FirstSec: .rela.plt LastSec: .dynamic +## Case B: the DT_PLTRELSZ (PLT size) has value of 0x269, what is too large, because PLT goes past the EOF. + +# RUN: yaml2obj --docnum=3 -DVAL=0x269 %s -o %t10b +# RUN: llvm-readobj --dyn-relocations %t10b 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t10b --check-prefix=INVALID-DT-PLTRELSZ2-LLVM --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-relocations %t10b 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t10b --check-prefix=INVALID-DT-PLTRELSZ2-GNU --implicit-check-not=warning: + +# INVALID-DT-PLTRELSZ2-LLVM: Dynamic Relocations { +# INVALID-DT-PLTRELSZ2-LLVM-NEXT: warning: '[[FILE]]': unable to read data at 0x78 of size 0x269 (DT_PLTRELSZ value): it goes past the end of the file of size 0x2e0 +# INVALID-DT-PLTRELSZ2-LLVM-NEXT: } + +# INVALID-DT-PLTRELSZ2-GNU: 'PLT' relocation section at offset 0x78 contains 617 bytes: +# INVALID-DT-PLTRELSZ2-GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend +# INVALID-DT-PLTRELSZ2-GNU-NEXT: warning: '[[FILE]]': unable to read data at 0x78 of size 0x269 (DT_PLTRELSZ value): it goes past the end of the file of size 0x2e0 + ## Show we print a warning when dumping dynamic relocations if there is no dynamic symbol table. # RUN: yaml2obj --docnum=4 %s -o %t11 # RUN: llvm-readobj --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=LLVM-NO-DYNSYM diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 0d53ab6..de8d290 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -126,9 +126,9 @@ template struct RelSymbol { /// the size, entity size and virtual address are different entries in arbitrary /// order (DT_REL, DT_RELSZ, DT_RELENT for example). struct DynRegionInfo { - DynRegionInfo(StringRef ObjName) : FileName(ObjName) {} - DynRegionInfo(const uint8_t *A, uint64_t S, uint64_t ES, StringRef ObjName) - : Addr(A), Size(S), EntSize(ES), FileName(ObjName) {} + DynRegionInfo(const Binary &Owner) : Obj(&Owner) {} + DynRegionInfo(const Binary &Owner, const uint8_t *A, uint64_t S, uint64_t ES) + : Addr(A), Size(S), EntSize(ES), Obj(&Owner) {} /// Address in current address space. const uint8_t *Addr = nullptr; @@ -137,8 +137,8 @@ struct DynRegionInfo { /// Size of each entity in the region. uint64_t EntSize = 0; - /// Name of the file. Used for error reporting. - StringRef FileName; + /// Owner object. Used for error reporting. + const Binary *Obj; /// Error prefix. Used for error reporting to provide more information. std::string Context; /// Region size name. Used for error reporting. @@ -151,6 +151,22 @@ struct DynRegionInfo { const Type *Start = reinterpret_cast(Addr); if (!Start) return {Start, Start}; + + const uint64_t Offset = + Addr - (const uint8_t *)Obj->getMemoryBufferRef().getBufferStart(); + const uint64_t ObjSize = Obj->getMemoryBufferRef().getBufferSize(); + + if (Size > ObjSize - Offset) { + reportWarning( + createError("unable to read data at 0x" + Twine::utohexstr(Offset) + + " of size 0x" + Twine::utohexstr(Size) + " (" + + SizePrintName + + "): it goes past the end of the file of size 0x" + + Twine::utohexstr(ObjSize)), + Obj->getFileName()); + return {Start, Start}; + } + if (EntSize == sizeof(Type) && (Size % EntSize == 0)) return {Start, Start + (Size / EntSize)}; @@ -165,7 +181,7 @@ struct DynRegionInfo { (" or " + EntSizePrintName + " (0x" + Twine::utohexstr(EntSize) + ")") .str(); - reportWarning(createError(Msg.c_str()), FileName); + reportWarning(createError(Msg.c_str()), Obj->getFileName()); return {Start, Start}; } }; @@ -296,8 +312,7 @@ private: ") + size (0x" + Twine::utohexstr(Size) + ") is greater than the file size (0x" + Twine::utohexstr(Obj.getBufSize()) + ")"); - return DynRegionInfo(Obj.base() + Offset, Size, EntSize, - ObjF.getFileName()); + return DynRegionInfo(ObjF, Obj.base() + Offset, Size, EntSize); } void printAttributes(); @@ -1927,7 +1942,7 @@ void ELFDumper::loadDynamicTable() { if (!DynamicPhdr && !DynamicSec) return; - DynRegionInfo FromPhdr(ObjF.getFileName()); + DynRegionInfo FromPhdr(ObjF); bool IsPhdrTableValid = false; if (DynamicPhdr) { // Use cantFail(), because p_offset/p_filesz fields of a PT_DYNAMIC are @@ -1943,7 +1958,7 @@ void ELFDumper::loadDynamicTable() { // Ignore sh_entsize and use the expected value for entry size explicitly. // This allows us to dump dynamic sections with a broken sh_entsize // field. - DynRegionInfo FromSec(ObjF.getFileName()); + DynRegionInfo FromSec(ObjF); bool IsSecTableValid = false; if (DynamicSec) { Expected RegOrErr = @@ -2005,10 +2020,8 @@ void ELFDumper::loadDynamicTable() { template ELFDumper::ELFDumper(const object::ELFObjectFile &O, ScopedPrinter &Writer) - : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()), - DynRelRegion(O.getFileName()), DynRelaRegion(O.getFileName()), - DynRelrRegion(O.getFileName()), DynPLTRelRegion(O.getFileName()), - DynamicTable(O.getFileName()) { + : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()), DynRelRegion(O), + DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O), DynamicTable(O) { // Dumper reports all non-critical errors as warnings. // It does not print the same warning more than once. WarningHandler = [this](const Twine &Msg) { @@ -2125,7 +2138,7 @@ void ELFDumper::parseDynamicTable() { // If we can't map the DT_SYMTAB value to an address (e.g. when there are // no program headers), we ignore its value. if (const uint8_t *VA = toMappedAddr(Dyn.getTag(), Dyn.getPtr())) { - DynSymFromTable.emplace(ObjF.getFileName()); + DynSymFromTable.emplace(ObjF); DynSymFromTable->Addr = VA; DynSymFromTable->EntSize = sizeof(Elf_Sym); DynSymFromTable->EntSizePrintName = ""; -- 2.7.4