Fix dockerfile.security.missing-user rules

[saghaulor]

• A USER directive can appear after the CMD or ENTRYPOINT directive and still be valid
• Updated sample dockerfiles with code comments
• Relates to Fixed the dockerfile.security.missing-user rule #3438

[0xDC0DE]

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

[0xDC0DE]

Suggested change

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

[0xDC0DE]

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)

[0xDC0DE]

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

[0xDC0DE]

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

[0xDC0DE]

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

[0xDC0DE]

Thanks for your contributions, @saghaulor ! These are meaningful updates, let's try to get that CI to pass so we can merge this PR!

[saghaulor]

@0xDC0DE thank you, I have returned from a long needed vacation, I'll try to make these changes soon.

[saghaulor]

@0xDC0DE I finally had a chance to look into this more. It seems that this metavariable ellipsis bug is what is preventing us from writing an optimal rule. I tried to write a rule like you specified and it didn't work. The missing-user.fixed.dockerfile ends up in the results, which should not happen.

❯ head -n20 missing-user.yaml
rules:
- id: missing-user
  patterns:
    - pattern: |
        CMD $...VARS
    - pattern-not-inside: |
        USER $USER
        ...
        CMD $...VARS
    - pattern-not-inside: |
        CMD $...VARS
        ...
        USER $USER
  fix: |
    USER non-root
    CMD $...VARS
  message: By not specifying a USER, a program in the container may run as 'root'. This is a security
    hazard. If an attacker can control a process running as root, they may have control over the container.
    Ensure that the last USER in a Dockerfile is a USER other than 'root'.
  severity: ERROR

❯ semgrep scan --config=missing-user.yaml


┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 14 files (only git-tracked) with 1 Code rule:

  CODE RULES
  Scanning 8 files.

  SUPPLY CHAIN RULES

  No rules to run.


  PROGRESS

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00


┌─────────────────┐
│ 2 Code Findings │
└─────────────────┘

    missing-user.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep --config localfile targets
           10┆ CMD semgrep --config localfile targets

    missing-user.fixed.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep
           11┆ CMD semgrep

If we change the rule to remove the metavariable ellipsis behavior, it works as expected, namely, the missing-user.fixed.dockerfile is no longer reporting a finding. Please note that I only changed the second pattern-not-inside clause to remove the metavariable ellipsis behavior as that is what would trigger in the fixed missing-user.fixed.dockerfile.

❯ head -n20 missing-user.yaml
rules:
- id: missing-user
  patterns:
    - pattern: |
        CMD $...VARS
    - pattern-not-inside: |
        USER $USER
        ...
        CMD $...VARS
    - pattern-not-inside: |
        CMD ...
        ...
        USER $USER
  fix: |
    USER non-root
    CMD $...VARS
  message: By not specifying a USER, a program in the container may run as 'root'. This is a security
    hazard. If an attacker can control a process running as root, they may have control over the container.
    Ensure that the last USER in a Dockerfile is a USER other than 'root'.
  severity: ERROR

❯ semgrep scan --config=missing-user.yaml


┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 14 files (only git-tracked) with 1 Code rule:

  CODE RULES
  Scanning 8 files.

  SUPPLY CHAIN RULES

  No rules to run.


  PROGRESS

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00


┌────────────────┐
│ 1 Code Finding │
└────────────────┘

    missing-user.dockerfile
   ❯❯❱ missing-user
          By not specifying a USER, a program in the container may run as 'root'. This is a security hazard.
          If an attacker can control a process running as root, they may have control over the container.
          Ensure that the last USER in a Dockerfile is a USER other than 'root'.

           ▶▶┆ Autofix ▶ USER non-root CMD semgrep --config localfile targets
           10┆ CMD semgrep --config localfile targets

This metavariable ellipsis bug also prevents the Semgrep engine from identifying incorrect patterns within the IDE.

The rule will trigger and highlight code when the Dockerfile specifies something like:

CMD semgrep

But not when it's something like:

CMD semgrep --severity=error

Therefore, I think we need to address the metavariable ellipsis bug before we can fix this rule for good. I believe that there is already an issue open here semgrep/semgrep#9109 regarding this bug. I believe this issue is also related semgrep/semgrep#9726

Please advise.

[0xDC0DE]

Thanks for digging into this, @saghaulor . I've pinged some of our engineers to see if they can bump the issues you've linked in priority, but it looks like they've been open for a while.