-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Fix incorrect offset in BPF sign extension LPE #14279
Conversation
The uid field of the cred struct is normally the second field, followed by the gid field. The first field is of type atomic_t, which has the size of an int. Since the size of an int is usually 4 bytes, the uid is normally located at an offset of 4 bytes from the start of the cred struct, and not 8. Since the uid also is int-sized, the code set test_uid to the gid, making the exploit fail for cases where uid != gid.
Thanks for the contribution @gblomqvist this makes a lot of sense. We will have to recompile your submission as part of it is a binary update and Rapid7 requires that all binaries are compiled by a Rapid7 employee, but otherwise this should be good to test and then merge if the tests pass. |
Oh okay, that's understandable. I couldn't find any official information on how you dealt with binaries, and judging by the conversation in the PR recommended in CONTRIBUTING.md I guessed that anyone could compile (although the compilation instructions there don't seem to correspond with how the provided binaries actually are compiled). I see how that would have been weird though. Let me know if you want me to do anything. |
@gblomqvist No problem its not obvious and your all good 👍 |
Ok initial tests look good and this appears to fix the issue described. Here is what this looks like after the patch is applied, testing on Ubuntu 16.04 x64 fresh install:
|
Yep confirmed this fix works as expected as here is what it looks like without the fix:
|
Landed 👍 Thanks for the contribution @gblomqvist and congrats on your first Metasploit contribution! Commit 65fcf67 was ninja committed in, and just is me recompiling the binary to meet Rapid7's policy that all binaries must be compiled by Rapid7 employees. Its not explained anywhere publicly as you mentioned so sorry for the confusion (its still a somewhat new change hence why this is probably the case). I'll write up some release notes for this shortly 👍 |
Original Release NotesA bug has been fixed in the |
Great, thanks! Just a small note regarding the release note, it says: Also, maybe that policy regarding binaries can be added to the CONTRIBUTING.md file? EDIT: I interpret it as if the policy is rather new. Could anyone compile binaries before that? And have you, the team of Rapid7 employees, recompiled all third-party binaries since then? |
@jmartin-r7 Would probably be the best person to ask r.e your latter question. As for your request, good spotting, I'll edit that up now 👍 |
@gblomqvist, the policy on binaries landing to the public repo has been the standard in metasploit-framework for years. Since the project will be disturbing the file directly from the repository, this ensures both of the following conditions:
I do like the suggestion about documenting the policy. We do have some Acceptance Guidelines in the wiki, however it does not yet have a section about how submitted binaries are validated. We often comment on the PR as a courtesy to let users know when we need to add a commit as part of the PR landing process. |
Release NotesFixed a bug in the |
The
uid
field of thecred
struct is normally the second field, followed by thegid
field (definition). The first field is of typeatomic_t
, which has the size of anint
. Since the size of anint
is usually 4 bytes, theuid
is normally located at an offset of 4 bytes from the start of thecred
struct, and not 8. Since theuid
also isint
-sized, the code previously settest_uid
to thegid
, making the exploit fail for cases whereuid != gid
.I discovered this issue after noticing a case where this exploit didn't work while the original exploit did.
The binary was compiled with
gcc -o exploit.out exploit.c
on a 64-bit system.Verification
msfconsole
use exploit/linux/local/bpf_sign_extension_priv_esc
set SESSION <ID>
where ID is the ID of a session with uid not necessarily equal to gid.run
getuid
to verify you got a root session.