Repository navigation
Conversation
getCaps() returned a raw pointer to a thread-local array, and the get/set helpers indexed it with CAP_TO_INDEX(cap) without checking the result against the _LINUX_CAPABILITY_U32S_3 elements that capget() and capset() exchange. Keep the capability sets in a caller-owned std::array instead, and look up the element for a capability through capEntry(), which rejects capabilities outside of the array. A capability which is out of range reads as not set, and requesting one fails initNs(), as an unpermitted capability already does. The highest capability in capNames is CAP_CHECKPOINT_RESTORE (40), so no current capability is out of range and behavior doesn't change: the jailed process gets the same capability sets as before for --cap (including CAP_BPF and CAP_CHECKPOINT_RESTORE, which live in the second element) and for --keep_caps. A future capability >= 64 added to the table now fails closed instead of reading and writing past the array. caps.cc now builds without clang -Wunsafe-buffer-usage warnings (5 before).
This branch has not been deployed
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.
getCaps() returned a raw pointer to a thread-local array, and the
get/set helpers indexed it with CAP_TO_INDEX(cap) without checking the
index against the two elements that capget()/capset() exchange.
This keeps the capability sets in a caller-owned std::array and looks
up a capability's element through a helper that rejects out-of-range
capabilities. An out-of-range capability reads as not set, and
requesting one makes initNs() fail, as an unpermitted capability
already does.
No capability in capNames is out of range today (the highest is
CAP_CHECKPOINT_RESTORE, 40), so behavior doesn't change. A capability
Testing:
make testgives the same results as master. CapInh, CapPrm,CapEff, CapBnd and CapAmb inside the jail are identical before and
after for: the default; --cap CAP_NET_RAW --cap CAP_SYS_ADMIN;
--keep_caps; --cap CAP_BPF --cap CAP_CHECKPOINT_RESTORE (second array
element); --disable_clone_newuser --cap CAP_NET_ADMIN; and
--user 99999 --cap CAP_KILL.