Fix shell injection in external_data Dockerfile curl RUN lines - #2616
Conversation
external_data URLs were embedded unescaped in shell RUN curl commands. Use the existing dockerfile_env_value filter for URL and destination paths.
The serving Dockerfile also contains a curl for uv bootstrap; scope the assertion to the external_data RUN line only.
cretz
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Left a couple of comments
| {%- if external_data_files %} | ||
| {% for url, dst in external_data_files %} | ||
| RUN mkdir -p {{ dst.parent }}; curl -L "{{ url }}" -o {{ dst }} | ||
| RUN mkdir -p {{ dst.parent | string | dockerfile_env_value }}; curl -L {{ url | dockerfile_env_value }} -o {{ dst | string | dockerfile_env_value }} |
There was a problem hiding this comment.
dockerfile_env_value implements buildkit's ENV grammar, but this is a shell RUN which backticks still substitute, so http://example.com/`cmd` gets through. shlex.quote() is probably a better approach
There was a problem hiding this comment.
You're right. Switched that line to a dockerfile_shell_value filter that is shlex.quote, and added a backtick case plus /bin/sh execution tests.
There was a problem hiding this comment.
Also, are a Baseten user? Have you been able to test this against Baseten SaaS platform (why or why not)?
There was a problem hiding this comment.
I use Truss locally to package models for serving. I have not run this change against Baseten SaaS. The bug is in Dockerfile generation on truss build, so I checked it there
I have signed up for Baseten previously but I am still in the waiting room. Thank you for reviewing my PR!
dockerfile_env_value leaves backticks live in /bin/sh, so a URL like http://example.com/`cmd` still executes during image build.
cretz
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
There was a problem hiding this comment.
Looks like CI is failing lint on this file
There was a problem hiding this comment.
Fixed. ruff format on that file, and skipped the /bin/sh tests on Windows CI.
Should be good to go now!
The /bin/sh execution tests need dash, which Windows CI does not have. Also stop requiring /app/data in the Dockerfile line; Path.resolve() turns that into a drive path on Windows.
What
Truss can download remote files at image build time via
external_datainconfig.yaml. The serving image builder turns each entry into a DockerfileRUNline that shells out tocurl:The
urlfield is interpolated into that shell command with no escaping beyond wrapping it in double quotes. If the URL contains", the shell treats everything after the quote as a new command. A config like this:generates:
malicious_commandruns duringtruss build/docker build, inside the build container, with the builder's permissions.This is a build-time injection in user-supplied config. It matters when the person running the build did not write every field in
config.yamlthemselves: a forked truss repo, a shared template, a third-party example, or a compromised config checked into source control.After the fix, the same URL stays inside one quoted argument:
Related to #2486, which fixes path traversal in the runtime download path (
download.py). This PR covers the separate Dockerfile build path.Why
I was trying Truss with
external_datato pull model weights from a URL, same pattern I use in other serving setups. Before pushing, I looked at the generated Dockerfile to see what actually runs during build. Thecurlline embeds the URL directly in a shellRUNwith no escaping, so I tried a URL with a"in it and confirmed it breaks out of the quoted argument.How
Apply the existing
dockerfile_env_valueJinja filter (already used forENV SERVER_START_CMD) to the URL, destination path, andmkdirtarget inserver.Dockerfile.jinja. That filter escapes",\, and$for double-quoted shell/ENV contexts. No new escaping helper.Testing
Added:
test_shell_injection_metacharacters_in_urlintruss/tests/util/test_jinja.pytest_external_data_url_shell_metacharacters_escaped_in_dockerfileintruss/tests/contexts/image_builder/test_serving_image_builder.py