-
Notifications
You must be signed in to change notification settings - Fork 38.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
NsenterWriter is an arbitrary shell-execution / path traversal attack waiting to happen #16971
Comments
A quick review indicated this is the only potential spot for this in the codebase right now but that could change. |
Touchpoint: containerized exec #13691 Also, if we have to support containerizing the kubelet on pre-1.10, we will need to keep this around :-/ @smarterclayton, do you think that will be a need? |
I think in this specific case you can just use |
Issues go stale after 30d of inactivity. Prevent issues from auto-closing with an If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/reopen |
@tallclair: you can't re-open an issue/PR unless you authored it or you are assigned to it. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Automatic merge from submit-queue (batch tested with PRs 66030, 65997). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Remove unused io util writer & volume host GetWriter() Cleanup unused code. Fixes #16971 **Release note**: ```release-note NONE ``` /kind cleanup /sig storage
EDIT: This is not a vulnerability right now, but it is a potential one.
While fixing #16969 we noticed that NsenterWriter has the potential for arbitrary shell-execution if a caller doesn't validate their paths. We need to ensure that is impossible by invoking
filepath.Clean
on the passed input, and then using%q
to verify that the shell we generate cannot be escaped. However, we may need to go even further (since bash + escaping == epic disaster).Docker 1.10 should have a mount propagation setting which allows --shared mount, at which point this code should die and be totally removed. We should leave the containerized code in experimental and put big security warnings around it until we have that.
I'd also like to figure out how we can have much stronger review policies around places where we execute bash code in the node, by requiring it go through a strict review process, and having specific helpers. I'd also like to put some automation in place to ensure we do not execute shell except through whitelisted paths.
@kubernetes/rh-platform-management @kubernetes/goog-node @liggitt
The text was updated successfully, but these errors were encountered: