Skip to content

Commit 3c39187

Browse files
authored
[ELF]Add overflow check to ELF note iterator (#160451)
Add overflow check to ELF note iterator to handle large `p_filesz` or `sh_size`, avoid accessing invalid memory. --------- Signed-off-by: Ruoyu Qiu <[email protected]>
1 parent 2cb5308 commit 3c39187

File tree

2 files changed

+76
-2
lines changed

2 files changed

+76
-2
lines changed

llvm/include/llvm/Object/ELF.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,8 @@ class ELFFile {
407407
Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const {
408408
assert(Phdr.p_type == ELF::PT_NOTE && "Phdr is not of type PT_NOTE");
409409
ErrorAsOutParameter ErrAsOutParam(Err);
410-
if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) {
410+
if (Phdr.p_offset + Phdr.p_filesz > getBufSize() ||
411+
Phdr.p_offset + Phdr.p_filesz < Phdr.p_offset) {
411412
Err =
412413
createError("invalid offset (0x" + Twine::utohexstr(Phdr.p_offset) +
413414
") or size (0x" + Twine::utohexstr(Phdr.p_filesz) + ")");
@@ -435,7 +436,8 @@ class ELFFile {
435436
Elf_Note_Iterator notes_begin(const Elf_Shdr &Shdr, Error &Err) const {
436437
assert(Shdr.sh_type == ELF::SHT_NOTE && "Shdr is not of type SHT_NOTE");
437438
ErrorAsOutParameter ErrAsOutParam(Err);
438-
if (Shdr.sh_offset + Shdr.sh_size > getBufSize()) {
439+
if (Shdr.sh_offset + Shdr.sh_size > getBufSize() ||
440+
Shdr.sh_offset + Shdr.sh_size < Shdr.sh_offset) {
439441
Err =
440442
createError("invalid offset (0x" + Twine::utohexstr(Shdr.sh_offset) +
441443
") or size (0x" + Twine::utohexstr(Shdr.sh_size) + ")");

llvm/unittests/Object/ELFTest.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "llvm/Object/ELF.h"
10+
#include "llvm/Object/ELFObjectFile.h"
11+
#include "llvm/ObjectYAML/yaml2obj.h"
12+
#include "llvm/Support/Error.h"
13+
#include "llvm/Support/YAMLTraits.h"
1014
#include "llvm/Testing/Support/Error.h"
1115
#include "gtest/gtest.h"
1216

@@ -310,3 +314,71 @@ TEST(ELFTest, Hash) {
310314
// presuming 32-bit long. Thus make sure that extra bit doesn't appear.
311315
EXPECT_EQ(hashSysV("ZZZZZW9p"), 0U);
312316
}
317+
318+
template <class ELFT>
319+
static Expected<ELFObjectFile<ELFT>> toBinary(SmallVectorImpl<char> &Storage,
320+
StringRef Yaml) {
321+
raw_svector_ostream OS(Storage);
322+
yaml::Input YIn(Yaml);
323+
if (!yaml::convertYAML(YIn, OS, [](const Twine &Msg) {}))
324+
return createStringError(std::errc::invalid_argument,
325+
"unable to convert YAML");
326+
return ELFObjectFile<ELFT>::create(MemoryBufferRef(OS.str(), "dummyELF"));
327+
}
328+
329+
TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) {
330+
using Elf_Shdr_Range = ELFFile<ELF64LE>::Elf_Shdr_Range;
331+
using Elf_Phdr_Range = ELFFile<ELF64LE>::Elf_Phdr_Range;
332+
333+
SmallString<0> Storage;
334+
Expected<ELFObjectFile<ELF64LE>> ElfOrErr = toBinary<ELF64LE>(Storage, R"(
335+
--- !ELF
336+
FileHeader:
337+
Class: ELFCLASS64
338+
Data: ELFDATA2LSB
339+
Type: ET_EXEC
340+
Machine: EM_X86_64
341+
ProgramHeaders:
342+
- Type: PT_NOTE
343+
FileSize: 0xffffffffffffff88
344+
FirstSec: .note.gnu.build-id
345+
LastSec: .note.gnu.build-id
346+
Sections:
347+
- Name: .note.gnu.build-id
348+
Type: SHT_NOTE
349+
AddressAlign: 0x04
350+
ShOffset: 0xffffffffffffff88
351+
Notes:
352+
- Name: "GNU"
353+
Desc: "abb50d82b6bdc861"
354+
Type: 3
355+
)");
356+
ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded());
357+
ELFFile<ELF64LE> Obj = ElfOrErr.get().getELFFile();
358+
359+
auto CheckOverflow = [&](auto &&PhdrOrShdr, uint64_t Offset, uint64_t Size) {
360+
Error Err = Error::success();
361+
Obj.notes(PhdrOrShdr, Err);
362+
363+
std::string ErrMessage;
364+
handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
365+
ErrMessage = EI.message();
366+
});
367+
368+
EXPECT_EQ(ErrMessage, ("invalid offset (0x" + Twine::utohexstr(Offset) +
369+
") or size (0x" + Twine::utohexstr(Size) + ")")
370+
.str());
371+
};
372+
373+
Expected<Elf_Phdr_Range> PhdrsOrErr = Obj.program_headers();
374+
EXPECT_FALSE(!PhdrsOrErr);
375+
for (Elf_Phdr_Impl<ELF64LE> P : *PhdrsOrErr)
376+
if (P.p_type == ELF::PT_NOTE)
377+
CheckOverflow(P, P.p_offset, P.p_filesz);
378+
379+
Expected<Elf_Shdr_Range> ShdrsOrErr = Obj.sections();
380+
EXPECT_FALSE(!ShdrsOrErr);
381+
for (Elf_Shdr_Impl<ELF64LE> S : *ShdrsOrErr)
382+
if (S.sh_type == ELF::SHT_NOTE)
383+
CheckOverflow(S, S.sh_offset, S.sh_size);
384+
}

0 commit comments

Comments
 (0)