Repository navigation
Conversation
hasCaps asks for LINUX_CAPABILITY_VERSION_3, for which the kernel always writes two CapUserData (caps 0-31 and 32-63, 24 bytes), but it passed a single one (12 bytes), so the kernel wrote 12 bytes past it. Pass two, and check the effective set of both. Signed-off-by: heppycat <qls13d@gmail.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the capability-check buffer sizing and effective-capability handling changes.
No open issues 🎉
What changed in this PR
Fixes a buffer overrun in the child process’s capability check by allocating enough space for the Linux capability version 3 response.
Changes:
- Uses a two-element
CapUserDataarray to accommodate both capability words written bycapget. - Checks both effective capability words, including capabilities above 31.
| File | Description |
|---|---|
pkg/child/child.go |
Corrects the capget buffer size and checks the full effective capability set. |
Review effort: Balanced
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.
hasCaps()callscapgetwithLINUX_CAPABILITY_VERSION_3and a singleunix.CapUserData. For versions 2 and 3 the kernel always writes two__user_cap_data_struct(caps 0-31 and 32-63, 24 bytes in total; seecapget(2)), so it writes 12 bytes pastdata.With this change the kernel's second struct shows up in the debug log, filled in (
rootlesskit --debug, amd64, kernel 7.2):Those are the bytes that currently go past the end of
data.I haven't seen this crash rootlesskit. The same mistake in Xray-core's WireGuard code (XTLS/Xray-core#7274) corrupted a saved return address on 32-bit ARM and crashed the process every time, so it isn't harmless in general.
The fix passes
[2]unix.CapUserDataand checks the effective set of both words. Capabilities above 31, such asCAP_BPFandCAP_CHECKPOINT_RESTORE, now count too, which they didn't before.Tested:
gofmtis clean, andgo vet ./pkg/child/passes for linux/amd64, arm64, arm and 386. It builds for amd64 and arm, and runs unprivileged on amd64.