From 4e91e509a0703fac7792b11a412c7461c6cb9c33 Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Wed, 24 Sep 2025 03:45:40 +0000 Subject: [PATCH 1/7] Add overflow check to ELF note iterator Signed-off-by: Ruoyu Qiu --- llvm/include/llvm/Object/ELF.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/Object/ELF.h b/llvm/include/llvm/Object/ELF.h index 0b362d389c177..59f63eb6b5bb6 100644 --- a/llvm/include/llvm/Object/ELF.h +++ b/llvm/include/llvm/Object/ELF.h @@ -407,7 +407,8 @@ class ELFFile { Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const { assert(Phdr.p_type == ELF::PT_NOTE && "Phdr is not of type PT_NOTE"); ErrorAsOutParameter ErrAsOutParam(Err); - if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) { + if (Phdr.p_offset + Phdr.p_filesz > getBufSize() || + Phdr.p_offset + Phdr.p_filesz < Phdr.p_offset) { Err = createError("invalid offset (0x" + Twine::utohexstr(Phdr.p_offset) + ") or size (0x" + Twine::utohexstr(Phdr.p_filesz) + ")"); @@ -435,7 +436,8 @@ class ELFFile { Elf_Note_Iterator notes_begin(const Elf_Shdr &Shdr, Error &Err) const { assert(Shdr.sh_type == ELF::SHT_NOTE && "Shdr is not of type SHT_NOTE"); ErrorAsOutParameter ErrAsOutParam(Err); - if (Shdr.sh_offset + Shdr.sh_size > getBufSize()) { + if (Shdr.sh_offset + Shdr.sh_size > getBufSize() || + Shdr.sh_offset + Shdr.sh_size < Shdr.sh_offset) { Err = createError("invalid offset (0x" + Twine::utohexstr(Shdr.sh_offset) + ") or size (0x" + Twine::utohexstr(Shdr.sh_size) + ")"); From a6300264e83ae77281324e1f9372cba3ed91f186 Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Thu, 25 Sep 2025 06:13:21 +0000 Subject: [PATCH 2/7] Add unittest Signed-off-by: Ruoyu Qiu --- llvm/unittests/Object/ELFTest.cpp | 77 +++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index faf855c09cfe8..9e7ee2072ada0 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -7,6 +7,10 @@ //===----------------------------------------------------------------------===// #include "llvm/Object/ELF.h" +#include "llvm/Object/ELFObjectFile.h" +#include "llvm/ObjectYAML/yaml2obj.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/YAMLTraits.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" @@ -310,3 +314,76 @@ TEST(ELFTest, Hash) { // presuming 32-bit long. Thus make sure that extra bit doesn't appear. EXPECT_EQ(hashSysV("ZZZZZW9p"), 0U); } + +template +static Expected> toBinary(SmallVectorImpl &Storage, + StringRef Yaml) { + raw_svector_ostream OS(Storage); + yaml::Input YIn(Yaml); + if (!yaml::convertYAML(YIn, OS, [](const Twine &Msg) {})) + return createStringError(std::errc::invalid_argument, + "unable to convert YAML"); + return ELFObjectFile::create(MemoryBufferRef(OS.str(), "dummyELF")); +} + +TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { + SmallString<0> Storage; + Expected> ElfOrErr = toBinary(Storage, R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_X86_64 +ProgramHeaders: + - Type: PT_NOTE + FileSize: 0xffffffffffffff88 + FirstSec: .note.gnu.build-id + LastSec: .note.gnu.build-id + +Sections: + - Name: .note.gnu.build-id + Type: SHT_NOTE + AddressAlign: 0x04 + ShOffset: 0xffffffffffffff88 + Notes: + - Name: "GNU" + Desc: "abb50d82b6bdc861" + Type: 3 +)"); + ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded()); + ELFFile Obj = ElfOrErr.get().getELFFile(); + + auto PhdrsOrErr = Obj.program_headers(); + EXPECT_FALSE(!PhdrsOrErr); + std::string ErrMessage; + Error PErr = Error::success(); + for (auto P : *PhdrsOrErr) { + if (P.p_type == ELF::PT_NOTE) { + Obj.notes(P, PErr); + handleAllErrors(std::move(PErr), [&](const ErrorInfoBase &EI) { + ErrMessage = EI.message(); + }); + EXPECT_EQ(ErrMessage, + ("invalid offset (0x" + Twine::utohexstr(P.p_offset) + + ") or size (0x" + Twine::utohexstr(P.p_filesz) + ")") + .str()); + } + } + + auto ShdrsOrErr = Obj.sections(); + EXPECT_FALSE(!ShdrsOrErr); + Error SErr = Error::success(); + for (auto S : *ShdrsOrErr) { + if (S.sh_type == ELF::SHT_NOTE) { + Obj.notes(S, SErr); + handleAllErrors(std::move(SErr), [&](const ErrorInfoBase &EI) { + ErrMessage = EI.message(); + }); + EXPECT_EQ(ErrMessage, + ("invalid offset (0x" + Twine::utohexstr(S.sh_offset) + + ") or size (0x" + Twine::utohexstr(S.sh_size) + ")") + .str()); + } + } +} From 93dbf91225eaf49951e49a77f1d1023d1ad4a83c Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Thu, 25 Sep 2025 06:42:13 +0000 Subject: [PATCH 3/7] Better code style Signed-off-by: Ruoyu Qiu --- llvm/unittests/Object/ELFTest.cpp | 46 +++++++++++++------------------ 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index 9e7ee2072ada0..f6777f3be617e 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -354,36 +354,28 @@ TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded()); ELFFile Obj = ElfOrErr.get().getELFFile(); + auto CheckOverflow = [&](auto &&PhdrOrShdr, uint64_t Offset, uint64_t Size) { + Error Err = Error::success(); + Obj.notes(PhdrOrShdr, Err); + + std::string ErrMessage; + handleAllErrors(std::move(Err), + [&](const ErrorInfoBase &EI) { ErrMessage = EI.message(); }); + + EXPECT_EQ(ErrMessage, ("invalid offset (0x" + Twine::utohexstr(Offset) + + ") or size (0x" + Twine::utohexstr(Size) + ")") + .str()); + }; + auto PhdrsOrErr = Obj.program_headers(); EXPECT_FALSE(!PhdrsOrErr); - std::string ErrMessage; - Error PErr = Error::success(); - for (auto P : *PhdrsOrErr) { - if (P.p_type == ELF::PT_NOTE) { - Obj.notes(P, PErr); - handleAllErrors(std::move(PErr), [&](const ErrorInfoBase &EI) { - ErrMessage = EI.message(); - }); - EXPECT_EQ(ErrMessage, - ("invalid offset (0x" + Twine::utohexstr(P.p_offset) + - ") or size (0x" + Twine::utohexstr(P.p_filesz) + ")") - .str()); - } - } + for (auto P : *PhdrsOrErr) + if (P.p_type == ELF::PT_NOTE) + CheckOverflow(P, P.p_offset, P.p_filesz); auto ShdrsOrErr = Obj.sections(); EXPECT_FALSE(!ShdrsOrErr); - Error SErr = Error::success(); - for (auto S : *ShdrsOrErr) { - if (S.sh_type == ELF::SHT_NOTE) { - Obj.notes(S, SErr); - handleAllErrors(std::move(SErr), [&](const ErrorInfoBase &EI) { - ErrMessage = EI.message(); - }); - EXPECT_EQ(ErrMessage, - ("invalid offset (0x" + Twine::utohexstr(S.sh_offset) + - ") or size (0x" + Twine::utohexstr(S.sh_size) + ")") - .str()); - } - } + for (auto S : *ShdrsOrErr) + if (S.sh_type == ELF::SHT_NOTE) + CheckOverflow(S, S.sh_offset, S.sh_size); } From 3a3b4ea4a2f81026690637e3418a8ee9d8901d71 Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Thu, 25 Sep 2025 06:47:28 +0000 Subject: [PATCH 4/7] Fix code format Signed-off-by: Ruoyu Qiu --- llvm/unittests/Object/ELFTest.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index f6777f3be617e..f9c993e0385b5 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -359,8 +359,9 @@ TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { Obj.notes(PhdrOrShdr, Err); std::string ErrMessage; - handleAllErrors(std::move(Err), - [&](const ErrorInfoBase &EI) { ErrMessage = EI.message(); }); + handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) { + ErrMessage = EI.message(); + }); EXPECT_EQ(ErrMessage, ("invalid offset (0x" + Twine::utohexstr(Offset) + ") or size (0x" + Twine::utohexstr(Size) + ")") From 00ff33a5a7ed79e22a7437c5761d4b562a272124 Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Fri, 26 Sep 2025 08:02:13 +0000 Subject: [PATCH 5/7] Optimize code style --- llvm/unittests/Object/ELFTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index f9c993e0385b5..e10c98f5b5952 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -368,13 +368,13 @@ TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { .str()); }; - auto PhdrsOrErr = Obj.program_headers(); + Expected::Elf_Phdr_Range> PhdrsOrErr = Obj.program_headers(); EXPECT_FALSE(!PhdrsOrErr); for (auto P : *PhdrsOrErr) if (P.p_type == ELF::PT_NOTE) CheckOverflow(P, P.p_offset, P.p_filesz); - auto ShdrsOrErr = Obj.sections(); + Expected::Elf_Shdr_Range> ShdrsOrErr = Obj.sections(); EXPECT_FALSE(!ShdrsOrErr); for (auto S : *ShdrsOrErr) if (S.sh_type == ELF::SHT_NOTE) From 646ed0bd978a56d5a4121c0689dd431ca9606317 Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Tue, 30 Sep 2025 03:45:39 +0000 Subject: [PATCH 6/7] Fix code style Signed-off-by: Ruoyu Qiu --- llvm/unittests/Object/ELFTest.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index e10c98f5b5952..d1d06c9945866 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -327,6 +327,9 @@ static Expected> toBinary(SmallVectorImpl &Storage, } TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { + using Elf_Shdr_Range = ELFFile::Elf_Shdr_Range; + using Elf_Phdr_Range = ELFFile::Elf_Phdr_Range; + SmallString<0> Storage; Expected> ElfOrErr = toBinary(Storage, R"( --- !ELF @@ -368,15 +371,15 @@ TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { .str()); }; - Expected::Elf_Phdr_Range> PhdrsOrErr = Obj.program_headers(); + Expected PhdrsOrErr = Obj.program_headers(); EXPECT_FALSE(!PhdrsOrErr); - for (auto P : *PhdrsOrErr) + for (Elf_Phdr_Impl P : *PhdrsOrErr) if (P.p_type == ELF::PT_NOTE) CheckOverflow(P, P.p_offset, P.p_filesz); - Expected::Elf_Shdr_Range> ShdrsOrErr = Obj.sections(); + Expected ShdrsOrErr = Obj.sections(); EXPECT_FALSE(!ShdrsOrErr); - for (auto S : *ShdrsOrErr) + for (Elf_Shdr_Impl S : *ShdrsOrErr) if (S.sh_type == ELF::SHT_NOTE) CheckOverflow(S, S.sh_offset, S.sh_size); } From c4c91cc33830d491276ccf84d3b5f25ea8239acf Mon Sep 17 00:00:00 2001 From: Ruoyu Qiu Date: Wed, 1 Oct 2025 12:22:34 +0000 Subject: [PATCH 7/7] Delete unnecessary blank line Signed-off-by: Ruoyu Qiu --- llvm/unittests/Object/ELFTest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/unittests/Object/ELFTest.cpp b/llvm/unittests/Object/ELFTest.cpp index d1d06c9945866..7c68ab5c8985f 100644 --- a/llvm/unittests/Object/ELFTest.cpp +++ b/llvm/unittests/Object/ELFTest.cpp @@ -343,7 +343,6 @@ TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) { FileSize: 0xffffffffffffff88 FirstSec: .note.gnu.build-id LastSec: .note.gnu.build-id - Sections: - Name: .note.gnu.build-id Type: SHT_NOTE