-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
Description of the false positive
This was originally reported for zizmorcore/zizmor#1705, please see the ticket for more context and an SSCCE. It's been explained that zizmor is showing that vulnerability report as a result of this entry in CodeQL:
| extensions: | |
| - addsTo: | |
| pack: codeql/actions-all | |
| extensible: actionsSinkModel | |
| data: | |
| - ["docker/build-push-action", "*", "input.context", "code-injection", "manual"] |
Code samples or links to source code
The full, step-by-step analysis was documented here: zizmorcore/zizmor#1705 (comment), but for the sake of clarity:
docker/build-push-actionpasses context as a string item in an array:- Creation of the array:
- Passing it further:
https://github.com/docker/build-push-action/blob/master/src/main.ts#L103-L109
- That last call to
Exec.getExecOutput(...)leads todocker/actions-toolkit, which still processes all args as an array:
https://github.com/docker/actions-toolkit/blob/main/src/exec.ts#L27-L30 - The implementation of
getExecOputpt(...)callsexec(...)in the same class: - The
exec(...)functions processes theargsarray, but only to extract the first element of it (the command -buildorbuildxdepending on the earlier setup), and passes theargsarray toToolRunnerclass, in the same library. - The implementation of
ToolRunneronly processes theargsvia_getSpawnArgsfunction, which still returns them as array and then uses Node.js built-inchild_process.spawn()(see here: https://github.com/actions/toolkit/blob/main/packages/exec/src/toolrunner.ts#L437-L441) to invoke docker.
The args are being passed down as an array through all these layers, and finally end up in a Node.js built-in. So, unless the're a vulnerability in Node.js child_process.spawn, passing the context as . --buildArg FOO=foo will not cause the FOO build argument to be passed to docker. Instead the whole thing will be treated as context path and cause docker build command to fail.