semgrep / semgrep-rules

Semgrep rules registry
https://semgrep.dev/registry
Other
773 stars 383 forks source link

dockerfile.security.missing-user has a false positive related to HEALTHCHECK CMD #3436

Open saghaulor opened 1 month ago

saghaulor commented 1 month ago

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

FROM some.registry/java/jre-17-corretto-amazonlinux-2023:latest

WORKDIR /service
EXPOSE 18252 18253

HEALTHCHECK --interval=30s --timeout=3s \
  CMD curl -sSfI --retry 0 --tcp-nodelay http://localhost:18253/actuator/health/liveness || exit 1

USER user

COPY ./scripts/entrypoint.sh /service/bin/
COPY build-artifact /service/bin/build-artifact.jar
ENTRYPOINT ["/service/bin/entrypoint.sh"]

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.

saghaulor commented 1 month ago

This rule will also create two findings if the Dockerfile looks like this:

FROM some.registry/java/jre-17-corretto-amazonlinux-2023:latest

WORKDIR /service
EXPOSE 18252 18253

HEALTHCHECK --interval=30s --timeout=3s \
  CMD curl -sSfI --retry 0 --tcp-nodelay http://localhost:18253/actuator/health/liveness || exit 1

COPY ./scripts/entrypoint.sh /service/bin/
COPY build-artifact /service/bin/build-artifact.jar
CMD ["java", "/service/bin/build-artifact.jar"]
USER user

Which is definitely not desirable. The rule should be that the USER directive is missing. That would eliminate all of these bugs.

saghaulor commented 1 month ago

The HEALTHCHECK .. CMD issue was fixed by https://github.com/semgrep/semgrep/commit/c697cdf772760110ad084138219042952d10dfcf

However, the issue of USER some-user appearing after ENTRYPOINT or CMD still incorrectly triggers the missing-user rules.