Security hardening: fix buffer handling and input validation#356
Open
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Open
Security hardening: fix buffer handling and input validation#356assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Conversation
## Comments for reviewers: ## Threat model: Local files that can only be edited by root are ## out-of-scope and considered trusted. emerg-shutdown.c: - Fix strtok separator mismatch in load_list(): continuation call hardcoded "," instead of using the caller's separator, breaking pipe-separated key alias parsing (e.g. KEY_A|KEY_B). - Fix FIFO permissions: mkfifo() used 0777, changed to 0600 to prevent unprivileged users from triggering emergency shutdown. - Fix out-of-bounds read in netlink parsing: use strnlen() bounded by remaining buffer length instead of unbounded strlen(). - Fix NULL dereference on empty --timeout= argument. - Add missing exit(0) after kill_system() in paranoid mode. - Fix print() writing NUL terminator to output. permission-hardener: - Anchor user/group existence checks to line boundaries to prevent substring matches (e.g. 'roo' matching 'root:'). - Anchor dpkg-statoverride path lookups to prevent substring path matches (e.g. '/usr/bin/su' matching '/usr/bin/sudo'). pam-info: - Add PAM_USER sanity check: reject control characters and values starting with '-' to prevent option injection into faillock. pam_faillock_not_if_x: - Remove set -x debug tracing from production PAM module to avoid leaking authentication details into system logs. build-fm-shim-backend: - Use atomic write pattern (compile to temp file, then mv) to prevent leaving a corrupted binary on interrupted compilation. postinst: - Add comments explaining that silent error handling for permission-hardener and update-grub is by design to avoid breaking APT. hide-hardware-info, emerg-shutdown (shell): - Add threat model comments documenting that root-owned config directories are trusted. https://claude.ai/code/session_01LvyeXrhG1t4pNmiHt7iAbi
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
2 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.
This PR addresses multiple security issues and improves robustness across the security-misc codebase.
Summary
Multiple security vulnerabilities and edge cases have been fixed, including buffer handling issues, input validation problems, and unsafe string operations. Additionally, several defensive programming improvements have been made to prevent potential exploitation vectors.
Key Changes
Buffer and String Handling:
print()function to not include null terminator in write length calculation, preventing off-by-one errorshw_monitor()strlen()calls withstrnlen()to prevent reading past buffer boundaries when processing truncated netlink messages0777to restrictive0600Input Validation:
fifo_monitor()to prevent null dereferencepam-infoto reject control characters and option injection attemptsstrtok()call to use correct separator variable instead of hardcoded commaRegex Pattern Matching:
permission-hardenerto line boundaries to prevent substring matches (e.g., 'roo' matching 'root')Build Process:
fm-shim-backendby writing to temporary file first, then moving into place to prevent corrupted binaries on interrupted buildsCode Quality:
exit(0)afterkill_system()call for clarityset -x) inpam_faillock_not_if_xto prevent leaking authentication details to system logsNotable Implementation Details
strnlen()with the remaining buffer length to safely determine entry length without reading past boundaries$'\n'syntax to match line boundaries and prevent substring matches in multi-line stringshttps://claude.ai/code/session_01LvyeXrhG1t4pNmiHt7iAbi