fix(dracut-install): bounds-check RUNPATH walk and ELF string derefs - #2622
Open
prabhakarpujeri wants to merge 2 commits into
Open
fix(dracut-install): bounds-check RUNPATH walk and ELF string derefs#2622prabhakarpujeri wants to merge 2 commits into
prabhakarpujeri wants to merge 2 commits into
Conversation
The PARSE_ELF_START macro checked only that e_shoff and e_shstrndx are individually < src_len, then indexed shdr[e_shstrndx].sh_offset into the mmap'ed file. Two classes of OOB reads remain on truncated or malformed ELFs: 1. The section header table extent is not validated: e_shnum section headers starting at shoff can extend past the end of the file, so iterating them reads past the map. 2. The section header string table (shstrtab) content extent isn't validated: sh_offset/sh_size come from the file, and the strcmp(&shstrtab[shdr[i].sh_name], ...) reads at attacker-controlled offsets, potentially past the mapping. Add the missing bounds checks: - PARSE_ELF_START now verifies the whole section table fits in the mapped file, then treats it as untrusted: a helper reads the shstrtab section's offset and size, validates both against src_len, and the section-name lookup checks the name offset and NUL termination inside the table before any strcmp. - DRUNPATH/DT_SONAME/DT_NEEDED walks now validate the shadow-section link index (sh_link < e_shnum) before dereferencing shdr[sh_link]. Verified by crafting five malformed ELF64 inputs to the new boundschecks and feeding them to the dependency resolution path: - Off-map shstrtab.sh_offset: segfault without patch, clean skip with - sh_name past table end: segfault without patch - No NUL within name's table window: segfault without patch - Section table extent truncated (F1) / sh_link past end (F6b): clean rejection with patch Regression control: -l dependency resolution of /bin/bash and /bin/ls yields byte-identical results before/after. Also verified syntax with and without HAVE_SYSTEMD (covers RESOLVE_DEPS_DLOPEN_FOR_BITS).
The FIND_LIBRARY_RUNPATH_FOR_BITS macro lacked the bounds checks its sibling macros (RESOLVE_DEPS_NEEDED_FOR_BITS, RESOLVE_DEPS_DLOPEN_FOR_BITS) have: - the .dynamic section's shdr[]-indexed pointer into the map - the per-iteration Elf_Dyn pointer in the DT_NULL-terminated walk - the interpreter string (PT_INTERP) and DT_SONAME/DT_NEEDED string, passed to strdup/strlen with no NUL-termination verification A malformed ELF could put a non-NUL-terminated runpath right at the end of the map (reads past it in strlen), or an interpreter offset exactly == src_len (off by > vs >=) that passed the old bounds check. Add a shared helper, elf_map_string(), that validates the offset is inside the map and the string is NUL-terminated there, and use it at every place the code dereferences a dynamic-entry/segment-list string. Mirror the per-iteration map-range checks in the RUNPATH walk that the sibling macros already carry. Depends on 0549d1c ("fix(dracut-install): validate ELF section-table bounds"), which introduced elf_sect_name() and the PARSE_ELF_START extent checks. Verified with valgrind on crafted malformed ELF64s (pre-patch SIGSEGV reading past the mmap; post-patch clean) and byte-for-byte identical dependency sets for real binaries (/bin/bash, /bin/ls, /usr/bin/ssh).
Collaborator
|
CC @chewi |
chewi
reviewed
Aug 20, 2026
| continue; \ | ||
| \ | ||
| Elf##B##_Dyn *dyn = (Elf##B##_Dyn *)((char *)map + ELF_BYTESWAP(B, shdr[i].sh_offset)); \ | ||
| if ((char *)dyn < (char *)map || (char *)dyn > (char *)map + src_len) \ |
Member
There was a problem hiding this comment.
I think void * rather than char * here and below would be equivalent but less confusing?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2615 (depends on its
PARSE_ELF_STARTextent checks andelf_sect_namehelper). Will be rebased after #2615 merges.Problem
FIND_LIBRARY_RUNPATH_FOR_BITS(insrc/install/dracut-install.c) walks the ELF.dynamicsection'sElf_Dynarray and pulls out the DT_RUNPATH / DT_RPATH strings to search for libraries. Unlike the sibling macrosRESOLVE_DEPS_NEEDED_FOR_BITSandRESOLVE_DEPS_DLOPEN_FOR_BITS, this walk validates onlye_shoff/e_shstrndx— the per-iteration pointers into the map are never bounds-checked. Additionally, in the interpreter/DT_SONAME/DT_NEEDEDcode paths the strings extracted with(char *)map + shdr[i].sh_offset + d->d_un.d_valare handed tostrlen/strdup/hashmap_get/expand_runpath/regexecwithout verifying they are NUL-terminated within the mapped file (a malformed ELF reading past the end of the map can hang on a never-terminatedstrcmp/strdup).Also found one off-by-boundary check in the interpreter path:
interpreter > (char *)map + src_lenaccepts the pointer at exactly one-past-end (should be>=).Fix
elf_map_string(map, src_len, offset): returns NULL if the offset is outside the map or the string there isn't NUL-terminated within the map. Used for the runpath string, interpreter string, DT_SONAME and DT_NEEDED strings.dynandd.Verification
Crafted 3 malformed ELF64 inputs exercising each new check; ran the real
dracut-install -lbinary built before/after, andvalgrindon the difference-maker:H7A2: DT_RPATH whose strtab pointer lands 4 bytes before map end, runpath not NUL-terminatedInvalid read of size 1→ SIGSEGVH7B2: PT_INTERP at exactlysrc_lenone-past-endH7C2: PT_INTERP in-map but non-terminated(The last two are safe in both versions only because no code path dereferences them — they're defensive/paranoia checks. The RUNPATH one is the reachable crash.)
Regression control:
-ldependency resolution of/bin/bash,/bin/ls, and/usr/bin/ssh— byte-identical 33-entry file list before vs after. H6 test set re-verified — the 6 malformed ELFs from #2615's verification table still all exit cleanly.