Template injection remediation when used as an action input

[chgl]

As of v1.9.0, the template-injection audit also triggers if an expandable variable is passed to another GitHub action as input.

For example, a reusable workflow provides build-context as an input string and passes it to the docker/build-push action:

      - name: test
        uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0
        with:
          context: ${{ inputs.build-context }}

(Taken from https://github.com/chgl/.github/blob/master/.github/workflows/standard-build.yaml#L178-L192).

Zizmor now remarks:

note[template-injection]: code injection via template expansion
   --> ./.github/workflows/standard-build.yaml:178:9
    |
178 |       - name: Build unit test image layer
    |         --------------------------------- note: this step
179 |         if: ${{ inputs.enable-build-test-layer == true }}
...
182 |         with:
183 |           context: ${{ inputs.build-context }}
    |           ------------------------------------ note: ${{ inputs.build-context }} may expand into attacker-controllable code
    |
    = note: audit confidence → Unknown

Is there any way to handle such cases? Or even ignore them explicitly?

[woodruffw]

Hey @chgl, thanks for opening a discussion.

As of v1.9.0, the template-injection audit also triggers if an expandable variable is passed to another GitHub action as input.

To qualify: zizmor doesn't flag every input, just ones that it thinks correspond to injectable code. In this particular case, docker/build-push-action's context: input is one that zizmor thinks is code-injectable, per our extraction of GitHub's CodeQL sinks:

zizmor/crates/zizmor/data/codeql-injection-sinks.json

Lines 9 to 13 in 45be0d6

	"docker/build-push-action",
	[
	"context"
	]
	],

That in turn was derived from CodeQL's model here:

https://github.com/github/codeql/blob/fcf6c3c4e83f127aea9ce5aed3e0ceb4feb65bc5/actions/ql/lib/ext/manual/docker_build-push-action.model.yml

However, from a look, I'm not 100% clear on why CodeQL thinks that this input is a code injection source -- context appears to influence some flags, but doesn't inject code directly. So this might be an upstream false positive from CodeQL's data.

TL;DR: This might be an upstream bug in CodeQL's data, which I'll be looking into. However, in the mean time, you could also manually ignore this with a # zizmor: ignore[template-injection] comment, e.g.:

      - name: test
        uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0
        with:
          context: ${{ inputs.build-context }} # zizmor: ignore[template-injection]

[woodruffw]

From looking a bit more: inputs.context can include a Handlebars expression, and those might be susceptible to code injection. So I wonder if that's why CodeQL has it manually marked.

[woodruffw]

For example: https://mahmoudsec.blogspot.com/2019/04/handlebars-template-injection-and-rce.html

[woodruffw]

Given the above, I think this is a true positive, albeit a confusing one (since the code execution in question is via a handlebars expression).

[chgl]

Thank you for the detailed explanation!

Is there any reasonable remediation for this? Something like running a script to first sanitize all input and then pass it to the action is the only idea I have... which seems quite involved.

[woodruffw]

Yeah, unfortunately this seems non-trivial to remediate 😞 -- an ignore comment might be right if you're confident that only you (or other privileged actors) can set the inputs.context field. Sanitizing would probably be the most appropriate general solution, but I agree about that being involved (and probably too involved to justify for a single input).

It might not be practical for them to change either, but ideally docker/build-push-action wouldn't use a full-fledged Handlebars expression here -- they could probably get away with a Mustache template instead, which wouldn't include the arbitrary code execution. But even that would be quite a long migration tail.

TL;DR: There unfortunately may not be a great general remediation for this that is also not exceedingly complex -- your best bet may be ignoring it.

[chgl]

Thanks again! Ignoring does seem like the best bet for now - still it's incredibly useful to at least know where to be cautious when passing inputs.

[mschoettle]

I actually ran into the same exact use case and tried to fix it via env as usual:

- name: Build and push Docker image
   id: push
   uses: docker/[email protected]
   env:
     CONTEXT: ${{ inputs.context }}
   with:
     context: ${{ env.CONTEXT }}

But it still gets flagged. Should this not fix it or is it still susceptible to the above mentioned code executed via handlebar expressions?