Gc #13
Open
Gc #13
Conversation
This file contains 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
o don't use malloc/free, allocate space on stack o don't use unsafe sprintf o use ARG_MAX as cmd size for clarity, cmd isn't just a filename o remove unsafe logging, ensure last points inside cmd
By doing so we may - move change of identity to the child, so no need to change it back - drop saved set-user/group-ID safely An attacker will gain no more than a process with its ids. As a side effect, the code that unsets environment variables becomes useless. - implement asynchronous wait for child later
An attacker could access the communication channel between su and Superuser otherwise.
Restructure the code a bit decreasing the cost of checks for non-CM ROMs
Just check whether ro.cm.version exists without getting its value nobody cares of anyway. The check_property function used here returns true if a property with given prefix exists. More than enough for CM and others. Certainly, the best way is to fix the nighmare called get_property. At least, those stupid strdup() shall be removed. If somebody is willing to fix this "code", he/she is welcomed.
Choose the value of the socket field in intents depending on the allow arg, because the socket_path argument is removed from send_intent. Use enum type for allow.
Pick up am exit code in the SIGCHLD handler. Kill am with request (first child) after a response has been received from Requestor, if that am hangs.
... so su can get its exit status
It's too high on the stack
It's shared between the SIGCHLD signal handler and normal flow. Its type is still pid_t, however, not sig_atomic_t.
<endian.h> defines htobe32 and like, not htonl we're using. Strictly speaking, we have to to include <arpa/inet.h>. su isn't aimed to be so portable though.
Corresponds with Superuser repo commit e16376e17a7680c50bdf626a73f46a8b60bd4793
See matching commit in Superuser repo
|
What is the reason for doing a merge like that? |
|
N/a |
shadiberck
approved these changes
Apr 17, 2024
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.
No description provided.