[llvm-objcopy] - Emit error and don't crash if program header reaches past end of file.

[grimar]

Summary

This is https://bugs.llvm.org/show_bug.cgi?id=42122.

If an object file has a size less than program header's file [offset + size]
(i.e. if we have overflow), llvm-objcopy crashes instead of reporting a
error.

The patch fixes this issue.

[grimar]

• Removed excessive props from yaml.

[jhenderson]

test/tools/llvm-objcopy/ELF/invalid-p_filesz.test

7 ↗

(On Diff #203117)

Could the numbers be in hex? I feel like those are more natural.

Also, there are a couple of typos. 1) FIleCheck has a capital 'I' in it spuriously, and "mailformed" should be "malformed".

I'd probably change the message slightly to say "with offset 0x1234 and file size ..." (because the program header is not at an offset, it has an offset field, and there are two sizes in program headers).

20 ↗

(On Diff #203117)

I think we need another test case here, where the file offset is outside the file entirely.

tools/llvm-objcopy/ELF/Object.cpp

1109

mailformed -> malformed (as noted above).

[grimar]

• Addressed review comments.

test/tools/llvm-objcopy/ELF/invalid-p_filesz.test

7 ↗

(On Diff #203117)

Yeah.. I guess I wanted to start from "program segment at offset ..."

[jhenderson]

test/tools/llvm-objcopy/ELF/invalid-p_filesz-p_offset.test

24

now -> now the
of program -> of the program

tools/llvm-objcopy/ELF/Object.cpp

1110

Maybe rather than "is malformed" it might be better to say "goes past the end of the file" or something similar.

[jakehehrlich]

"goes past the end of the file" is the phrasing that I've seen GNU tools use and gets more to the point so I agree with James. Other than that this looks good to me!

[grimar]

• Addressed review comments.

[jhenderson]

LGTM.

test/tools/llvm-objcopy/ELF/invalid-p_filesz-p_offset.test

+## In this case, we have a program header with a file size that
+## overflows the binary size. Check llvm-objcopy doesn't crash
+## and report this error properly.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1.o
+# RUN: not llvm-objcopy %t1.o 2>&1 | FileCheck %s --check-prefix=ERR1
+# ERR1: error: program header with offset 0x1b8 and file size 0x100000 is malformed
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
+ProgramHeaders:
+  - Type:        PT_LOAD
+    FileSize:    0x100000
+    Sections:    
+      - Section: .foo
+
+## A similar case, but now p_offset property of program header is too large.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2.o
+# RUN: not llvm-objcopy %t2.o 2>&1 | FileCheck %s --check-prefix=ERR2
+# ERR2: error: program header with offset 0x100000 and file size 0x1 is malformed
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
+    Size: 1
+ProgramHeaders:
+  - Type:        PT_LOAD
+    Offset:      0x100000
+    FileSize:    1
+    Sections:
+      - Section: .foo

tools/llvm-objcopy/ELF/Object.cpp

if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
         }
     }
   }
 }
 
 template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
   uint32_t Index = 0;
   for (const auto &Phdr : unwrapOrError(ElfFile.program_headers())) {
+    if (Phdr.p_offset + Phdr.p_filesz > ElfFile.getBufSize())
+      error("program header with offset 0x" + Twine::utohexstr(Phdr.p_offset) +
+            " and file size 0x" + Twine::utohexstr(Phdr.p_filesz) +
+            " is malformed");
+
     ArrayRef<uint8_t> Data{ElfFile.base() + Phdr.p_offset,
                            (size_t)Phdr.p_filesz};
     Segment &Seg = Obj.addSegment(Data);
     Seg.Type = Phdr.p_type;
     Seg.Flags = Phdr.p_flags;
     Seg.OriginalOffset = Phdr.p_offset;
     Seg.Offset = Phdr.p_offset;
     Seg.VAddr = Phdr.p_vaddr;