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

Presence of spaces in a command is not escaped and results in an incorrect Dockerfile #192

Open
kit-ty-kate opened this issue Nov 20, 2023 · 2 comments · May be fixed by #194
Open

Presence of spaces in a command is not escaped and results in an incorrect Dockerfile #192

kit-ty-kate opened this issue Nov 20, 2023 · 2 comments · May be fixed by #194

Comments

@kit-ty-kate
Copy link
Contributor

Example:

from "alpine" @@
run "ls\n/"

results in the incorrect Dockerfile:

FROM alpine
RUN ls
/

instead, each \n characters should be escaped using \ as per https://docs.docker.com/engine/reference/builder/#format

@MisterDA MisterDA linked a pull request Nov 21, 2023 that will close this issue
@MisterDA
Copy link
Contributor

I have mixed feelings about the bug report. I think there's a case for considering that the shell syntax requires the escape, not the Dockerfile, and that the newline should be escaped in the original string.
I've drafted a patch for the problem, though. I'm also wondering where the escape should apply, only RUN instructions, or to a lot more places?

@kit-ty-kate
Copy link
Contributor Author

I don't mind if the escape are not added but I think the library should check that the string given as input is welformed and is not going to escape its scope and create a new section. e.g.

run "%s" input_from_user

if input_from_users = "true\nA-NEW-MALICIOUS-SECTION", then it would create a potentially dangerous Dockerfile:

RUN true
A-NEW-MALICIOUS-SECTION

I also think this check should be done in all the places a raw input is outputted (most instructions i suspect)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants