[llvm-objdump][ELF] Add Section size check. (#86612)

[cabbaken]

This change make the check of the section size to avoid crashing of llvm-objdump when processing misformatted elf file.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-binary-utilities

Author: Cabbaken (cabbaken)

Changes

This change make the check of the section size to avoid crashing of llvm-objdump when processing misformatted elf file.

---

Full diff: https://github.com/llvm/llvm-project/pull/124470.diff

1 Files Affected:

• (modified) llvm/tools/llvm-objdump/ELFDump.cpp (+12)

diff --git a/llvm/tools/llvm-objdump/ELFDump.cpp b/llvm/tools/llvm-objdump/ELFDump.cpp
index e9e5b059f1786e..83699f8267575c 100644
--- a/llvm/tools/llvm-objdump/ELFDump.cpp
+++ b/llvm/tools/llvm-objdump/ELFDump.cpp
@@ -221,6 +221,13 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
   std::string TagFmt = "  %-" + std::to_string(MaxLen) + "s ";
 
   outs() << "\nDynamic Section:\n";
+  auto DynamicSectionOrErr = Elf.getSection(ELF::SHT_DYNAMIC);
+  if (!DynamicSectionOrErr) {
+    reportWarning(toString(DynamicSectionOrErr.takeError()), Obj.getFileName());
+    return;
+  }
+  const auto StringTableSize = (*DynamicSectionOrErr)->sh_size;
+
   for (const typename ELFT::Dyn &Dyn : DynamicEntries) {
     if (Dyn.d_tag == ELF::DT_NULL)
       continue;
@@ -235,6 +242,11 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
       Expected<StringRef> StrTabOrErr = getDynamicStrTab(Elf);
       if (StrTabOrErr) {
         const char *Data = StrTabOrErr->data();
+        if (Dyn.getVal() > StringTableSize) {
+          reportWarning("Invalid string table offset for section .dynstr",
+                        Obj.getFileName());
+          continue;
+        }
         outs() << format(TagFmt.c_str(), Str.c_str()) << Data + Dyn.getVal()
                << "\n";
         continue;

[brad0]

@MaskRay

[MaskRay]

We need a test. See #86612 (comment) Also check out https://llvm.org/docs/CodingStandards.html#error-and-warning-messages for the format of diagnostics. You could read other diagnostics in the file as well.

[cabbaken]

I will try to fix it in a few days. Thanks for your friendly reply!

[jh7370]

It looks like your change breaks a number of llvm-objdump tests. Please see the pre-merge checks on this PR. It's possible these tests are doing something that is technically invalid, but wasn't important for the thing being tested, or it could be that you've broken some other behaviour with this change. Please remember to run the existing llvm-objdump tests locally before pushing additional changes, to make sure your code is up to scratch before asking for review.

[cabbaken]

This is my first time using the test system, and I'm new to the community. I appreciate your response and will make sure to conduct local testing in the future. Thank you for your guidance!