Skip to content

CVE-2024-9407: validate "bind-propagation" flag settings#5761

Merged
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
nalind:validate-bind-propagation
Oct 1, 2024
Merged

CVE-2024-9407: validate "bind-propagation" flag settings#5761
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
nalind:validate-bind-propagation

Conversation

@nalind
Copy link
Copy Markdown
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 <nalin@redhat.com>
@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
Copy Markdown
Member

mheon commented Oct 1, 2024

LGTM

Copy link
Copy Markdown
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
Copy Markdown
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

@openshift-ci
Copy link
Copy Markdown
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

Details 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
Copy Markdown
Member

mheon commented Oct 1, 2024

/lgtm

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

mheon commented Oct 1, 2024

/cherry-pick v1.37

@openshift-cherrypick-robot
Copy link
Copy Markdown

@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.

Details

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
@openshift-cherrypick-robot
Copy link
Copy Markdown

@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

Details

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
Copy Markdown
Member

mheon commented Oct 1, 2024

Argh
/cherry-pick release-v1.37

@openshift-cherrypick-robot
Copy link
Copy Markdown

@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

Details

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
Copy Markdown
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 <mheon@redhat.com>
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 <mheon@redhat.com>
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jan 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved kind/bug Categorizes issue or PR as related to a bug. lgtm locked - please file new issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants