server: fail if HOME variable has a newline #6450
Conversation
openshift-ci
bot
added
do-not-merge/work-in-progress
haircommander
force-pushed
the
homedir-drop
branch
from
ba647f1 to
4c438b4
Compare
openshift-ci
bot
added
the
approved
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6450 +/- ##
==========================================
- Coverage 43.58% 43.57% -0.01%
==========================================
Files 123 123
Lines 14390 14392 +2
==========================================
Hits 6272 6272
- Misses 7438 7440 +2
Partials 680 680 |
|
/retest |
haircommander
force-pushed
the
homedir-drop
branch
from
4c438b4 to
7f3a9ab
Compare
to prevent CVE-2022-4318 Signed-off-by: Peter Hunt~ <pehunt@redhat.com>
haircommander
force-pushed
the
homedir-drop
branch
from
7f3a9ab to
41dca27
Compare
|
/retest |
haircommander
changed the title
openshift-ci
bot
added
release-note
|
/override ci/prow/ci-rhel-integration |
|
@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-conmonrs, ci/prow/ci-rhel-integration 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. |
|
@cri-o/cri-o-maintainers PTAL |
|
@umohnani8 @mrunalp @kolyshkin PTAL |
| @@ -201,6 +201,9 @@ func setupContainerUser(ctx context.Context, specgen *generate.Generator, rootfs | |||
| for _, env := range specgen.Config.Process.Env { | |||
| if strings.HasPrefix(env, "HOME=") { | |||
| homedir = strings.TrimPrefix(env, "HOME=") | |||
| if idx := strings.Index(homedir, `\n`); idx > -1 { | |||
| return fmt.Errorf("invalid HOME environment; newline not allowed") | |||
There was a problem hiding this comment.
Instead of failing, can we continue by dropping \n from the variable?
There was a problem hiding this comment.
if the value is set to a path with a newline because the path genuinely has a newline, then the HOME won't be set correctly and the container will fail more nefariously. I think it'd be better to fail quickly and notify the user why, rather than let the homedir be set wrong
There was a problem hiding this comment.
Oh ok, makes sense. Thanks.
|
LGTM |
|
/test e2e-gcp-ovn |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, mrunalp, sohankunkerkar 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 |
|
/override ci/prow/ci-rhel-integration |
|
@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-rhel-integration 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. |
|
/override ci/prow/e2e-gcp-ovn |
|
@haircommander: Overrode contexts on behalf of haircommander: ci/prow/e2e-gcp-ovn 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. |
|
/retest |
|
/override ci/prow/ci-rhel-integration |
|
@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-conmonrs, ci/prow/ci-rhel-integration 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. |
|
/override ci/prow/ci-rhel-integration |
|
@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-conmonrs, ci/prow/ci-rhel-integration 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. |
|
/cherry-pick release-1.25 |
|
@haircommander: new pull request created: #6524 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. |
Signed-off-by: Peter Hunt~ pehunt@redhat.com
What type of PR is this?
/kind deprecation
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?