commit 93d81944ed41a74693a5f38a7fba2690eca9a376
author Elliott Hughes <enh@google.com> Mon Jan 13 07:56:11 2025 -0800
committer Elliott Hughes <enh@google.com> Mon Jan 13 07:56:11 2025 -0800
tree 9e05007bd2929db63fb8129a1287af4dd4695562
parent 19584059da9cef30ea5b7bdd5915ba5b45d58878

Further harden the pad note parsing code.

Bug: http://b/386192920
Test: cd /data/fuzz/arm64/ElfReader_fuzzer && ./ElfReader_fuzzer corpus/
Change-Id: I8788cf87b0684bd3b97cd15b1fabb0fa72ac8f55

libc/bionic/elf_note.cpp

diff --git a/libc/bionic/elf_note.cpp b/libc/bionic/elf_note.cpp
index efe3844..28a400a 100644
--- a/libc/bionic/elf_note.cpp
+++ b/libc/bionic/elf_note.cpp
@@ -42,10 +42,12 @@
 
   ElfW(Addr) p = note_addr;
   ElfW(Addr) note_end = p + phdr_note->p_memsz;
-  while (p + sizeof(ElfW(Nhdr)) <= note_end) {
+  while (p < note_end) {
     // Parse the note and check it's structurally valid.
     const ElfW(Nhdr)* note = reinterpret_cast<const ElfW(Nhdr)*>(p);
-    p += sizeof(ElfW(Nhdr));
+    if (__builtin_add_overflow(p, sizeof(ElfW(Nhdr)), &p) || p >= note_end) {
+      return false;
+    }
     const char* name = reinterpret_cast<const char*>(p);
     if (__builtin_add_overflow(p, __builtin_align_up(note->n_namesz, 4), &p)) {
       return false;

linker/linker_phdr.cpp

diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index 1e36f5e..086c867 100644
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -766,22 +766,23 @@
       continue;
     }
 
-    // If the PT_NOTE extends beyond the file. The ELF is doing something
-    // strange -- obfuscation, embedding hidden loaders, ...
-    //
-    // It doesn't contain the pad_segment note. Skip it to avoid SIGBUS
-    // by accesses beyond the file.
-    off64_t note_end_off = file_offset_ + phdr->p_offset + phdr->p_filesz;
-    if (note_end_off > file_size_) {
-      continue;
+    // Reject notes that claim to extend past the end of the file.
+    off64_t note_end_off = file_offset_;
+    if (__builtin_add_overflow(note_end_off, phdr->p_offset, &note_end_off) ||
+        __builtin_add_overflow(note_end_off, phdr->p_filesz, &note_end_off) ||
+        phdr->p_filesz != phdr->p_memsz ||
+        note_end_off > file_size_) {
+      DL_ERR("\"%s\": NT_ANDROID_TYPE_PAD_SEGMENT note runs off end of file",
+             name_.c_str());
+      return false;
     }
 
-    // note_fragment is scoped to within the loop so that there is
-    // at most 1 PT_NOTE mapped at anytime during this search.
+    // We scope note_fragment to within the loop so that there is
+    // at most one PT_NOTE mapped at any time.
     MappedFileFragment note_fragment;
-    if (!note_fragment.Map(fd_, file_offset_, phdr->p_offset, phdr->p_memsz)) {
+    if (!note_fragment.Map(fd_, file_offset_, phdr->p_offset, phdr->p_filesz)) {
       DL_ERR("\"%s\": PT_NOTE mmap(nullptr, %p, PROT_READ, MAP_PRIVATE, %d, %p) failed: %m",
-             name_.c_str(), reinterpret_cast<void*>(phdr->p_memsz), fd_,
+             name_.c_str(), reinterpret_cast<void*>(phdr->p_filesz), fd_,
              reinterpret_cast<void*>(page_start(file_offset_ + phdr->p_offset)));
       return false;
     }
@@ -795,7 +796,7 @@
     }
 
     if (note_hdr->n_descsz != sizeof(ElfW(Word))) {
-      DL_ERR("\"%s\" NT_ANDROID_TYPE_PAD_SEGMENT note has unexpected n_descsz: %u",
+      DL_ERR("\"%s\": NT_ANDROID_TYPE_PAD_SEGMENT note has unexpected n_descsz: %u",
              name_.c_str(), reinterpret_cast<unsigned int>(note_hdr->n_descsz));
       return false;
     }