[llvm] [ELF]Add overflow check to ELF note iterator (PR #160451)
Ruoyu Qiu via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 24 23:48:38 PDT 2025
https://github.com/cabbaken updated https://github.com/llvm/llvm-project/pull/160451
>From 4e91e509a0703fac7792b11a412c7461c6cb9c33 Mon Sep 17 00:00:00 2001
From: Ruoyu Qiu <cabbaken at outlook.com>
Date: Wed, 24 Sep 2025 03:45:40 +0000
Subject: [PATCH 1/4] Add overflow check to ELF note iterator
Signed-off-by: Ruoyu Qiu <cabbaken at outlook.com>
---
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 <cabbaken at outlook.com>
Date: Thu, 25 Sep 2025 06:13:21 +0000
Subject: [PATCH 2/4] Add unittest
Signed-off-by: Ruoyu Qiu <cabbaken at outlook.com>
---
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 <class ELFT>
+static Expected<ELFObjectFile<ELFT>> toBinary(SmallVectorImpl<char> &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<ELFT>::create(MemoryBufferRef(OS.str(), "dummyELF"));
+}
+
+TEST(ELFObjectFileTest, ELFNoteIteratorOverflow) {
+ SmallString<0> Storage;
+ Expected<ELFObjectFile<ELF64LE>> ElfOrErr = toBinary<ELF64LE>(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<ELF64LE> 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 <cabbaken at outlook.com>
Date: Thu, 25 Sep 2025 06:42:13 +0000
Subject: [PATCH 3/4] Better code style
Signed-off-by: Ruoyu Qiu <cabbaken at outlook.com>
---
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<ELF64LE> 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 <cabbaken at outlook.com>
Date: Thu, 25 Sep 2025 06:47:28 +0000
Subject: [PATCH 4/4] Fix code format
Signed-off-by: Ruoyu Qiu <cabbaken at outlook.com>
---
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) + ")")
More information about the llvm-commits
mailing list