Skip to content

[llvm-objdump][ELF] Ensure offset to verdaux entry array does not go past size #115284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
25 changes: 25 additions & 0 deletions llvm/test/tools/llvm-objdump/ELF/invalid-verdef.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# RUN: yaml2obj %s -o %t
# RUN: llvm-objdump -p %t 2>&1 | FileCheck --check-prefix=BROKEN-VERDEF -DFILE=%t %s

## Ensure we emit a warning when vd_aux offset field is invalid.

# BROKEN-VERDEF: Version definitions:
# BROKEN-VERDEF-NEXT: warning: '[[FILE]]': corrupted section: vd_aux value 69 in section verdef points past end of the section

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_DYN
Sections:
- Name: .gnu.version_d
Type: SHT_GNU_verdef
Entries:
- Version: 1
Flags: 0
VersionNdx: 0
VDAux: 69
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still could do with a comment saying how it's invalid (i.e. what it should be).

Names:
- VERSION_1
DynamicSymbols: []
...
17 changes: 13 additions & 4 deletions llvm/tools/llvm-objdump/ELFDump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ template <typename ELFT> class ELFDumper : public Dumper {
void printProgramHeaders();
void printSymbolVersion();
void printSymbolVersionDependency(const typename ELFT::Shdr &Sec);
void printSymbolVersionDefinition(const typename ELFT::Shdr &Shdr,
ArrayRef<uint8_t> Contents,
StringRef StrTab);
};
} // namespace

Expand Down Expand Up @@ -380,9 +383,9 @@ void ELFDumper<ELFT>::printSymbolVersionDependency(
}

template <class ELFT>
static void printSymbolVersionDefinition(const typename ELFT::Shdr &Shdr,
ArrayRef<uint8_t> Contents,
StringRef StrTab) {
void ELFDumper<ELFT>::printSymbolVersionDefinition(
const typename ELFT::Shdr &Shdr, ArrayRef<uint8_t> Contents,
StringRef StrTab) {
outs() << "\nVersion definitions:\n";

const uint8_t *Buf = Contents.data();
Expand All @@ -398,6 +401,12 @@ static void printSymbolVersionDefinition(const typename ELFT::Shdr &Shdr,
<< format("0x%08" PRIx32 " ", (uint32_t)Verdef->vd_hash);

const uint8_t *BufAux = Buf + Verdef->vd_aux;
if (BufAux > Contents.end()) {
reportWarning("corrupted section: vd_aux value " + Twine(Verdef->vd_aux) +
" in section verdef points past end of the section",
Obj.getFileName());
Comment on lines +405 to +407
Copy link
Collaborator

Choose a reason for hiding this comment

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

I went with 'vd_aux value %val in section verdef points past end of the section, corrupted section,' as I don't think it’s that relevant to provide the exact byte count by which the aux entry went out of bounds. I added 'corrupted section' consistently with the other warnings.

I just searched for the string "corrupted section" and haven't found any relevant reference. What other warnings are you trying to be consistent with?

Please use the describe method (in Object\ELF.h) in place of "in section verdef" to be consistent with other recent messages added/changed in llvm-readelf.

The idea of including the section size is that it could be the size that is malformed, not the vd_aux value (for whatever reason). ELFDumper.cpp in llvm-readelf has a number of examples of this sort of message pattern (search for "past the end"). There may be others too.

break;
}
uint16_t VerdauxIndex = 0;
while (BufAux) {
auto *Verdaux = reinterpret_cast<const typename ELFT::Verdaux *>(BufAux);
Expand Down Expand Up @@ -430,7 +439,7 @@ template <class ELFT> void ELFDumper<ELFT>::printSymbolVersion() {
if (Shdr.sh_type == ELF::SHT_GNU_verneed)
printSymbolVersionDependency(Shdr);
else
printSymbolVersionDefinition<ELFT>(Shdr, Contents, StrTab);
printSymbolVersionDefinition(Shdr, Contents, StrTab);
}
}

Expand Down
Loading