-
Notifications
You must be signed in to change notification settings - Fork 414
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
dockerfile.security.missing-user has a false positive related to HEALTHCHECK CMD #3436
Open
1 of 3 tasks
Labels
bug
Something isn't working
Comments
This rule will also create two findings if the Dockerfile looks like this:
Which is definitely not desirable. The rule should be that the |
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Jul 26, 2024
- The previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` There will still be false positives if CMD is not indented on the newline - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - There is still a bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Partially fixes semgrep#3436
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Jul 26, 2024
- The previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` There will still be false positives if CMD is not indented on the newline - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - There is still a bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Cleaned up some of the test files to remove invalid syntax like calling CMD twice - Partially fixes semgrep#3436
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Jul 26, 2024
- The previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` There will still be false positives if CMD is not indented on the newline - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - There is still a bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Cleaned up some of the test files to remove invalid syntax like calling CMD twice - Partially fixes semgrep#3436
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Jul 28, 2024
- Fixed a bug where the previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` and ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` It doesn't really make sense to flag on the CMD sub-directive of the HEALTHCHECK directive since there's very little chance that the application could be compromised via the HEALTHCHECK and then gain root access. This false positive creates a lot of noise and therefore we're addressing it. - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - Fixed the bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Cleaned up some of the test files to remove invalid syntax like calling CMD twice - Partially fixes semgrep#3436
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Jul 28, 2024
- Fixed a bug where the previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` and ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` It doesn't really make sense to flag on the CMD sub-directive of the HEALTHCHECK directive since there's very little chance that the application could be compromised via the HEALTHCHECK and then gain root access. This false positive creates a lot of noise and therefore we're addressing it. - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - Fixed the bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Cleaned up some of the test files to remove invalid syntax like calling CMD twice - Fixes semgrep#3436
saghaulor
added a commit
to saghaulor/semgrep-rules
that referenced
this issue
Aug 1, 2024
- Fixed a bug where the previous version of this rule would have false positives on ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` and ``` HEALTHCHECK ... \ CMD ... ENTRYPOINT ... ``` It doesn't really make sense to flag on the CMD sub-directive of the HEALTHCHECK directive since there's very little chance that the application could be compromised via the HEALTHCHECK and then gain root access. This false positive creates a lot of noise and therefore we're addressing it. - There was a separate rule for ENTRYPOINT, which doesn't really make sense, since CMD and ENTRYPOINT can be used in the same Dockerfile, as per https://docs.docker.com/reference/dockerfile/#exec-form-entrypoint-example Therefore, the rule was removed - Fixed the bug that will create two findings for a Dockerfile like this ``` FROM busybox ENTRYPOINT ["some-command"] CMD ["--some-arg"] ``` - The autofix arguments have changed because technically it doesn't matter where in the Dockerfile the USER directive is specified, insofar as the last specified USER is non-root. Previously, the autofix would attempt to add the USER directive above the CMD or ENTRYPOINT directives. However, since either or both of these can appear, we're not going to specify the CMD or ENTRYPOINT directive in the fix. - Cleaned up some of the test files to remove invalid syntax like calling CMD twice - Fixes semgrep#3436
The However, the issue of |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
dockerfile.security.missing-user has a false positive related to HEALTHCHECK CMD. It triggers for Dockerfiles that do not have a CMD directive and only use an ENTRYPOINT directive.
To Reproduce
Expected behavior
This rule should not trigger because a non-root user is specified.
Priority
How important is this to you?
Additional Context
I don't think that this rule should trigger in the first place as this isn't the CMD directive, it's part of the HEALTHCHECK. I believe the intent of this rule is to prevent services to run as root so as to minimize exposure if the service is compromised. I'm not sure how you take advantage of the HEALTHCHECK calling curl with root permissions to cause problems.
There is another missing-user rule for ENTRYPOINT directive. This is odd because the CMD and ENTRYPOINT directives can both be used in a Dockerfile and this is valid. There is even a documented example as such. I would think that this should be one rule given this fact.
Lastly, my understanding is that any ENTRYPOINT or CMD directive uses the last declared user in the Dockerfile. I don't believe the USER directive is required to be declared before the ENTRYPOINT or CMD directive. If I understand both of these rules correctly, they assume that the USER directive must be declared before the ENTRYPOINT and CMD directive.
The text was updated successfully, but these errors were encountered: