Skip to content
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

CVE-2019-1002101: kubectl fix potential directory traversal #75037

Merged
merged 1 commit into from
Mar 6, 2019

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Mar 6, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
Fixes panic in kubectl cp command

Special notes for your reviewer:
/assign @tallclair @liggitt

Does this PR introduce a user-facing change?:

Fix panic in kubectl cp command

Update from Brandon @philips of the Kubernetes Security Commitee:

A security issue was discovered with the Kubernetes kubectl cp command that could enable a directory traversal replacing or deleting files on a user’s workstation. The issue is High severity and upgrading kubectl to Kubernetes 1.11.9, 1.12.7, 1.13.5, and 1.14.0 is encouraged to fix this issue.

Am I vulnerable?

Run kubectl version --client and if it does not say client version 1.11.9, 1.12.7, 1.13.5, and 1.14.0 or newer you are running a vulnerable version.

How do I upgrade?

Follow installation instructions here https://kubernetes.io/docs/tasks/tools/install-kubectl/

Not all instructions will provide up to date kubectl versions at the time of this announcement. So, always confirm with kubectl version.

Vulnerability Details

The kubectl cp command allows copying files between containers and the user machine. To copy files from a container, Kubernetes creates a tar inside the container, copies it over the network, and kubectl unpacks it on the user’s machine.

If the tar binary in the container is malicious, it could run any code and output unexpected, malicious results. An attacker could use this to write files to any path on the user’s machine when kubectl cp is called, limited only by the system permissions of the local user.

Since fixing CVE-2018-1002100, the untar function calls the cp.go:clean to strip path traversals. However, that function can both create and follow symbolic links.

See #75037 for details.

Thank you

Thank you to the reporter Ariel Zelivansky of Twistlock for identifying the issue, Maciej Szulik, Tim Pepper, and the patch release managers for the coordination in making this release.

Thank You,

Brandon on behalf of the Kubernetes Product Security Committee

@k8s-ci-robot k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 6, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 6, 2019
@soltysh
Copy link
Contributor Author

soltysh commented Mar 6, 2019

/priority important-soon
/sig cli

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 6, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@soltysh
Copy link
Contributor Author

soltysh commented Mar 6, 2019

/test pull-kubernetes-integration

@soltysh
Copy link
Contributor Author

soltysh commented Mar 6, 2019

/test pull-kubernetes-e2e-gce-100-performance

Copy link
Member

@tallclair tallclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Thanks!

linkname := header.Linkname
// error is returned if linkname can't be made relative to destFile,
// but relative can end up being ../dir that's why we also need to
// verify if relative path is the same after Clean-ing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
// verify if relative path is the same after Clean-ing
// verify if relative path is the same after removing backticks

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 6, 2019
@k8s-ci-robot k8s-ci-robot merged commit 4706389 into kubernetes:master Mar 6, 2019
// but relative can end up being ../dir that's why we also need to
// verify if relative path is the same after Clean-ing
relative, err := filepath.Rel(destFile, linkname)
if path.IsAbs(linkname) && (err != nil || relative != stripPathShortcuts(relative)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check be separated into a util class so that other calls to os.Symlink() can utilize ?

@tedyu
Copy link
Contributor

tedyu commented Mar 6, 2019

Reading the untarAll() function, I think the

                       defer outFile.Close()

on line 483 is redundant with the call on line 487.

Created PR #75074

@soltysh soltysh deleted the cp_bug branch March 7, 2019 09:07
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 12, 2019
k8s-ci-robot added a commit that referenced this pull request Mar 12, 2019
…7-upstream-release-1.12

Automated cherry pick of #75037: Fix panic in kubectl cp command
k8s-ci-robot added a commit that referenced this pull request Mar 18, 2019
…7-upstream-release-1.13

Automated cherry pick of #75037: Fix panic in kubectl cp command
k8s-ci-robot added a commit that referenced this pull request Mar 21, 2019
…7-upstream-release-1.11

Automated cherry pick of #75037: Fix panic in kubectl cp command
@philips philips changed the title Fix panic in kubectl cp command kubectl fix potential directory traversal - CVE-2019-1002101 Mar 28, 2019
@liggitt liggitt changed the title kubectl fix potential directory traversal - CVE-2019-1002101 CVE-2019-1002101: kubectl fix potential directory traversal Mar 28, 2019
@liggitt liggitt added this to the v1.14 milestone Jun 10, 2019
honkiko pushed a commit to honkiko/kubernetes that referenced this pull request Dec 5, 2019
kubectl cp potential directory traversal - CVE-2019-11246

kubectl cp potential directory traversal - CVE-2019-11246

kubernetes#75037
kubernetes#76788

See merge request !53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubectl area/security cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants