-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use safe_mode when flag is set on boot for OS X #7
Conversation
this caused this test to usually fail.
@@ -27,7 +27,7 @@ static bool pop(const void **buf, const void *end, | |||
*buf += sizeof(*op); | |||
*opc = op->opc; | |||
if ((size_t) (end - *buf) <= op->namelen || | |||
((const char *) buf)[op->namelen] != '\0') | |||
((const char *) *buf)[op->namelen] != '\0') | |||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without dereferencing buf
, you would be checking if the namelen bytes after the pointer was equal to zero rather than the data.
which should never happen
@@ -259,6 +273,8 @@ static int hook_posix_spawn_generic(__typeof__(posix_spawn) *old, | |||
if (newp != newp_orig) | |||
*newp++ = ':'; | |||
newp = stpcpy(newp, dylib_to_add); | |||
} else { | |||
newp = stpcpy(newp, "\0"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without the terminal operator, adjacent data would bleed into our string due to the memcpy(newp, p, next - p)
where next, in the case of the last library would be p + strlen(p)
which excludes '\0'
(wondering whether he was especially tired when writing this code or whether all his code is this bad) The uninitialized bool is correct, though. |
Right, thanks for looking at my code! Now that I'm actually working on this - I added the critical bug fixes from this PR in slightly modified form, which I will push once I check that they actually work. The Safe Mode thing makes sense, and I'll merge it if you fix it up as I requested. |
done (and squashed) |
we want to compare against p, not next
I guess I might as well merge the commits as is since it's not like my commit history isn't total shit, but thanks to Dolphin I'm used to being a stickler on PRs, and that is not rebased properly. :p |
When the user boots into safe mode with the -x flag or otherwise on OS X, substitute should be disabled. Also, there's a commit in here which makes the bootstrap stuff build no matter what (mostly so I could test on OS X, which I'm still in the process of setting up a VM to do so), I don't know if that goes according to your plan for this project, though.
I haven't tested this bit of code in the context of substitute, but I can affirm that it otherwise works as far as detecting safe mode.
Also, this has a bug fix for bundle-loader.c in which the test for the char after namelen characters in the name is equal to '\0' would usually fail.