Fix buffer overrun in NT_GNU_PROPERTY_TYPE_0 parser (#538)

* Fix buffer overrun in NT_GNU_PROPERTY_TYPE_0 parser

The `iter_notes` method has code to parse `NT_GNU_PROPERTY_TYPE_0` type notes.
The contents of the note are interpreted as an array of
`elffile.structs.Elf_Prop`s.

There was a bug where it would keep on parsing from the stream until the end of
the *segment or section*. This is only correct if the note would be the last in
the segment/section. In general, it should stop parsing until it reaches the
end of the note's data buffer.

This PR fixes this bug.

Fixes: #534

* Add comment explaining n_descsz

elftools/elf/notes.py

@@ -52,7 +52,10 @@ def iter_notes(elffile, offset, size):
         elif note['n_type'] == 'NT_GNU_PROPERTY_TYPE_0':
             off = offset
             props = []
-            while off < end:
+            # n_descsz contains the size of the note "descriptor" (the data payload),
+            # excluding padding. See "Note Section" in https://refspecs.linuxfoundation.org/elf/elf.pdf
+            current_note_end = offset + note['n_descsz']
+            while off < current_note_end:
                 p = struct_parse(elffile.structs.Elf_Prop, elffile.stream, off)
                 off += roundup(p.pr_datasz + 8, 2 if elffile.elfclass == 32 else 3)
                 props.append(p)

test/test_notes.py

@@ -1,10 +1,24 @@
-import unittest
 import os
+import unittest
 
 from elftools.elf.elffile import ELFFile
 from elftools.elf.sections import NoteSection
 
+
 class TestNotes(unittest.TestCase):
+    def test_note_after_gnu_property_type_note(self):
+        with ELFFile.load_from_path(os.path.join('test', 'testfiles_for_unittests', 'note_after_gnu_property', 'main.elf')) as elf:
+            note_sections = [section for section in elf.iter_sections() if isinstance(section, NoteSection)]
+            # There's only one note section in this file:
+            self.assertEqual(len(note_sections), 1)
+            notes = list(note_sections[0].iter_notes())
+            # There are 2 notes in this section:
+            self.assertEqual(len(notes), 2)
+            # The first note is the GNU_PROPERTY_TYPE_0 note:
+            self.assertEqual(notes[0].n_type, 'NT_GNU_PROPERTY_TYPE_0')
+            # It should only have two Elf_Props (and not attempt to parse the note after it as Elf_Props):
+            self.assertEqual(len(notes[0].n_desc), 2)
+
     def test_note_segment_with_8_byte_alignment(self):
         with ELFFile.load_from_path(os.path.join('test', 'testfiles_for_unittests', 'note_with_segment_padding', 'main.elf')) as elf:
             note_sections = [section for section in elf.iter_sections() if isinstance(section, NoteSection)]

test/testfiles_for_unittests/note_after_gnu_property/Makefile

@@ -0,0 +1,2 @@
+main.elf: main.c link.ld
+	gcc -O0 main.c -T link.ld -Wl,--build-id=none -o main.elf

test/testfiles_for_unittests/note_after_gnu_property/link.ld

@@ -0,0 +1,34 @@
+PHDRS
+{
+  elf_headers PT_LOAD FILEHDR PHDRS FLAGS(PF_R) ;
+  text PT_LOAD FLAGS(PF_R | PF_X) ;
+  data PT_LOAD FLAGS(PF_R | PF_W) ;
+  bss PT_LOAD FLAGS(PF_R | PF_W) ;
+  note PT_NOTE FLAGS(PF_R) ;
+}
+
+SECTIONS
+{
+  .text : {
+    *(.text .text.* .gnu.linkonce.t.*)
+  } :text
+
+  .rela.dyn : {
+    *(.rela.dyn)
+  } :text
+
+  .bss . (NOLOAD): {
+    *(.bss)
+    *(COMMON)
+  } :bss
+
+  .note : ALIGN(8) {
+    KEEP(*(.note.gnu*))
+    KEEP(*(.note.custom))
+  } :text :note
+
+  /DISCARD/ : {
+    *(.note.ABI-tag)
+  }
+}
+

test/testfiles_for_unittests/note_after_gnu_property/main.c

@@ -0,0 +1,28 @@
+#include <link.h>
+
+// NOTE: This is a minimal test fixture that creates an ELF file with a note
+// segment that has a single NOTE segment with a NT_GNU_PROPERTY_TYPE_0,
+// followed by a custom note. It's used in a regression test for a buffer
+// overrun bug in the parsing of the NT_GNU_PROPERTY_TYPE_0.
+
+struct elf_note {
+  ElfW(Nhdr) nhdr;  // header: 12 bytes
+  char name[4];     // name buffer: 2 bytes + 2 bytes padding
+  uint8_t data[8];  // data buffer: 8 bytes
+};
+
+__attribute__((section(".note.custom"), aligned(8)))
+__attribute__((used))
+const struct elf_note note = {
+    .nhdr = {
+        .n_namesz = 4,
+        .n_descsz = 8,
+        .n_type = 0,
+    },
+    .name = {'H', 'i', '\0'},
+    .data = {},
+};
+
+int main() {
+  return 0;
+}