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

Fix dockerfile.security.missing-user rules #3448

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dockerfile/security/missing-user-entrypoint.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ RUN pip3 install semgrep
# ruleid: missing-user-entrypoint
ENTRYPOINT semgrep -f p/xss

# TODO: metavar bug
# ok: missing-user-entrypoint
# TODO: metavar ellipses bug, this should be a finding but is a false negative
# ruleid: missing-user-entrypoint
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

For failing testcases that are related to the engine instead of the rule, we also have todook and todoruleid. This allows our tests to pass for now, and the rule in its current state to be published. But also allows our developers to verify what the intended behaviour is. If they make an engine update that fixes this problem, they will update the test syntax.

Suggested change
# TODO: metavar ellipses bug, this should be a finding but is a false negative
# ruleid: missing-user-entrypoint
# TODO: metavar ellipses bug, this should be a finding but is a false negative
# todoruleid: missing-user-entrypoint

ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]
5 changes: 3 additions & 2 deletions dockerfile/security/missing-user-entrypoint.fixed.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ RUN git clone https://github.com/returntocorp/semgrep
RUN pip3 install semgrep

# ruleid: missing-user-entrypoint
USER non-root
ENTRYPOINT semgrep -f p/xss

# TODO: metavar bug
# TODO: metavar ellipses bug
# ok: missing-user-entrypoint
ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]
Comment on lines 13 to 14
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# ok: missing-user-entrypoint
ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]
# todoruleid: missing-user-entrypoint
ENTRYPOINT ["semgrep", "--config", "localfile", "targets"]


USER non-root
1 change: 1 addition & 0 deletions dockerfile/security/missing-user-entrypoint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ rules:
- pattern: |
ENTRYPOINT $...VARS
- pattern-not-inside: |
...
USER $USER
...
Comment on lines +7 to 9
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this can be helped, but starting a pattern with an ellipses is very slow! (I think we have CI checks to not allow this)

Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid a starting ellipses, maybe we could do something like this?

    - pattern: |
        ENTRYPOINT $...VARS
    - pattern-not-inside:
        USER $USER
        ...
        ENTRYPOINT $...VARS
    - pattern-not-inside:
        CMD $...VARS
        ...
        ENTRYPOINT $USER

fix: |
Expand Down
7 changes: 2 additions & 5 deletions dockerfile/security/missing-user.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@ FROM busybox
RUN git clone https://github.com/returntocorp/semgrep
RUN pip3 install semgrep

# ruleid: missing-user
CMD semgrep -f p/xss

# ruleid: missing-user
CMD semgrep --config localfile targets

# TODO: metavar ellipses bug
# ok: missing-user
# TODO: metavar ellipses bug, this should be a failure but is a false negative
# ruleid: missing-user
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# TODO: metavar ellipses bug, this should be a failure but is a false negative
# ruleid: missing-user
# TODO: metavar ellipses bug, this should be a failure but is a false negative
# todoruleid: missing-user

CMD ["semgrep", "--version"]
7 changes: 2 additions & 5 deletions dockerfile/security/missing-user.fixed.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ FROM busybox
RUN git clone https://github.com/returntocorp/semgrep
RUN pip3 install semgrep

# ruleid: missing-user
USER non-root
CMD semgrep -f p/xss

# ruleid: missing-user
USER non-root
CMD semgrep --config localfile targets

# TODO: metavar ellipses bug
# ok: missing-user
CMD ["semgrep", "--version"]
CMD ["semgrep", "--config", "localfile", "targets"]
USER non-root
1 change: 1 addition & 0 deletions dockerfile/security/missing-user.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ rules:
- pattern: |
CMD $...VARS
- pattern-not-inside: |
...
USER $USER
...
Comment on lines +7 to 9
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid a starting ellipses, maybe we could do something like this?

    - pattern: |
        CMD $...VARS
    - pattern-not-inside:
        USER $USER
        ...
        CMD $...VARS
    - pattern-not-inside:
        CMD $...VARS
        ...
        USER $USER

fix: |
Expand Down
Loading