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-2024-9407: validate "bind-propagation" flag settings #5761

Merged

Conversation

nalind
Copy link
Member

@nalind nalind commented Oct 1, 2024

What type of PR is this?

/kind bug

What this PR does / why we need it:

Validate that the value for the "bind-propagation" flag when handling "bind" and "cache" mounts in buildah run or in RUN instructions is one of the values that we would accept without the "bind-propagation=" prefix.

How to verify it

New integration tests!

Which issue(s) this PR fixes:

Addresses CVE-2024-9407.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

CVE-2024-9407: validate that the value for the "bind-propagation" flag
when handling "bind" and "cache" mounts in `buildah run` or in RUN
instructions is one of the values that we would accept without the
"bind-propagation=" prefix.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@openshift-ci openshift-ci bot added kind/bug Categorizes issue or PR as related to a bug. approved labels Oct 1, 2024
@mheon
Copy link
Member

mheon commented Oct 1, 2024

LGTM

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +108 to +111
default:
return newMount, "", fmt.Errorf("%v: %q: %w", argName, argValue, errBadMntOption)
case "shared", "rshared", "private", "rprivate", "slave", "rslave":
// this should be the relevant parts of the same list of options we accepted above
Copy link
Member

Choose a reason for hiding this comment

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

tiny nit, but reading this I find it strange that the default case is first.
It seems to be common to place the default case last, anyhow no reason to block or repush

Copy link
Contributor

openshift-ci bot commented Oct 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, nalind

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

@mheon
Copy link
Member

mheon commented Oct 1, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Oct 1, 2024
@mheon
Copy link
Member

mheon commented Oct 1, 2024

/cherry-pick v1.37

@openshift-cherrypick-robot

@mheon: once the present PR merges, I will cherry-pick it on top of v1.37 in a new PR and assign it to you.

In response to this:

/cherry-pick v1.37

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-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit a518f88 into containers:main Oct 1, 2024
32 checks passed
@openshift-cherrypick-robot

@mheon: cannot checkout v1.37: error checking out "v1.37": exit status 1 error: pathspec 'v1.37' did not match any file(s) known to git

In response to this:

/cherry-pick v1.37

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-sigs/prow repository.

@mheon
Copy link
Member

mheon commented Oct 1, 2024

Argh
/cherry-pick release-v1.37

@openshift-cherrypick-robot

@mheon: cannot checkout release-v1.37: error checking out "release-v1.37": exit status 1 error: pathspec 'release-v1.37' did not match any file(s) known to git

In response to this:

Argh
/cherry-pick release-v1.37

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-sigs/prow repository.

@mheon
Copy link
Member

mheon commented Oct 1, 2024

/cherry-pick release-1.37

@nalind nalind deleted the validate-bind-propagation branch October 2, 2024 14:42
mheon added a commit to mheon/libpod that referenced this pull request Oct 4, 2024
Similar to github.com/containers/buildah/pull/5761 but not
security critical as Podman does not have an expectation that
mounts are scoped (the ability to write a --mount option is
already the ability to mount arbitrary content into the container
so sneaking arbitrary options into the mount doesn't have
security implications). Still, bad practice to let users inject
anything into the mount command line so let's not do that.

Signed-off-by: Matt Heon <[email protected]>
mheon added a commit to mheon/libpod that referenced this pull request Oct 7, 2024
Similar to github.com/containers/buildah/pull/5761 but not
security critical as Podman does not have an expectation that
mounts are scoped (the ability to write a --mount option is
already the ability to mount arbitrary content into the container
so sneaking arbitrary options into the mount doesn't have
security implications). Still, bad practice to let users inject
anything into the mount command line so let's not do that.

Signed-off-by: Matt Heon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved kind/bug Categorizes issue or PR as related to a bug. lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants