fix: some security stuff - #53
Merged
Merged
Conversation
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
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.
Some security stuff over the C code. I found two issues myself and got curious and asked DeepSeek V4 Pro for more input.
What I found was the PID check without upper bound which caused truncation. I then checked and was also present in the GUI obviously. What DeepSeek found (alongside a couple of false positives):
/proc/pid/comm, which any process can set to arbitrary bytes viaprctl(PR_SET_NAME).pidnl listprinted them raw, so a process could inject ANSI/OSC sequences into the admin's terminal (same class as CVE-2023-4016 inps). Control characters are now stripped when reading the name.exe_path. (I actually tested myself and I couldn't believe lol)/run/pidnlwithout checking what it was:statfollows symlinks, and there was no ownership/type check beforeunlink/bindon the socket. Now it requires a real root-owned directory vialstat, or fails hard.