Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions llvm/include/llvm/Object/ELF.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) + ")");
Expand Down Expand Up @@ -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) + ")");
Expand Down
70 changes: 70 additions & 0 deletions llvm/unittests/Object/ELFTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -310,3 +314,69 @@ 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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: delete this blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

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 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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd avoid auto in these tests to make it clearer what the type is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of PhdrsOrErr is Expected<ELFFile<ELF64LE>::Elf_Phdr_Range>, which is a bit verbose here. Since the getBuildID code uses auto, I followed the same style.
I can adjust this if needed.

EXPECT_FALSE(!PhdrsOrErr);
for (auto P : *PhdrsOrErr)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still using auto here and in the similar loop below. You could probably add a couple of using declarations to avoid repeating the full ELFFile<ELF64LE>::Elf_Phdr_Range and Shdr equivalent.

if (P.p_type == ELF::PT_NOTE)
CheckOverflow(P, P.p_offset, P.p_filesz);

auto ShdrsOrErr = Obj.sections();
EXPECT_FALSE(!ShdrsOrErr);
for (auto S : *ShdrsOrErr)
if (S.sh_type == ELF::SHT_NOTE)
CheckOverflow(S, S.sh_offset, S.sh_size);
}