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

prevent terminal injection when sending to terminal #6150

Closed
JanZerebecki opened this issue Nov 1, 2020 · 7 comments
Closed

prevent terminal injection when sending to terminal #6150

JanZerebecki opened this issue Nov 1, 2020 · 7 comments

Comments

@JanZerebecki
Copy link

JanZerebecki commented Nov 1, 2020

I did this

> echo -e '#!/bin/sh\n\necho "evil!"\nexit 0\n\033[2Aecho "Hello World!"\n' > evil.sh
> curl file:/home/jan_automation/evil.sh
#!/bin/sh

echo "Hello World!"
exit 0

Classical terminal injection (see https://www.infosecmatter.com/terminal-escape-injection/ ). Everyone using a terminal is expected to know not to do this on untrusted or random content. It is very user unfriendly and it is possible to do better.

Even careful humans make mistakes, so this can happen unintentionally.

Quick way to show that this is also the case for headers in case of a curious reader:

> echo -ne "HTTP/1.0 200 OK\r\nEvil: yes\033[2Ano\r\n\r\nEverything is fine.\nHave some tea.\n" | nc -l -p 8080 &
> curl -v localhost:8080

I expected the following

> curl file:/home/jan_automation/evil.sh
#!/bin/sh

echo "evil!"
exit 0
␛[2Aecho "Hello World!"
> curl file:/home/jan_automation/evil.sh | cat
#!/bin/sh

echo "Hello World!"
exit 0
> curl --disable-terminal-filter file:/home/jan_automation/evil.sh
#!/bin/sh

echo "Hello World!"
exit 0
> curl --enable-terminal-filter file:/home/jan_automation/evil.sh
#!/bin/sh

echo "evil!"
exit 0
␛[2Aecho "Hello World!"

There seems to be no way around detecting a terminal and only doing this when output is sent there, so as not to break many scripts using curl that redirect output to a file or pipe. An argument would be needed to disable this always and one to enable it always. There are more places where output will need to be filtered, like response headers when using --verbose.

Would such a fix be acceptable?

curl/libcurl version

all of them

operating system

most of them

@jay
Copy link
Member

jay commented Nov 1, 2020

Twenty years ago you could send a command (essentially prepend to the next command line) to be executed the next time the user hit enter. We've come a long way.

@bagder
Copy link
Member

bagder commented Nov 1, 2020

Would such a fix be acceptable?

What fix are you suggesting? What you see as a bug here is used as a feature by many...

@JanZerebecki
Copy link
Author

I added 3 more expected examples to the first message. Does that answer your question?

@bagder
Copy link
Member

bagder commented Nov 1, 2020

Not completely. Can you explain in words what the "terminal-filter" does?

@bagder
Copy link
Member

bagder commented Nov 19, 2020

So far this sounds like a vaguely stated feature-request. I will close soon unless with get more content to it.

@emilengler
Copy link
Contributor

Maybe he means something like this? https://www.php.net/manual/de/function.escapeshellcmd.php

@JanZerebecki
Copy link
Author

No, that is for escaping bytes that a shell would interpret (not doing this allows an attacker to execute code). That is not what happens here. I'm talking about bytes that a terminal would interpret. Since most terminals got fixed, they don't allow this to execute code. Now not escaping this will merely allow an attacker to confuse a human viewer, e.g. to hide something from a human. I should have picked the untrusted HTTP headers seen with --verbose as the main example, because it is mixed with the trusted output from the progress bar and terminal colors.

$ echo -ne "HTTP/1.0 200 OK\r\nEvil: yes\033[2A a\r\n\r\nEverything is fine.\nHave some tea.\n" | nc -l -p 8080 &
$ curl -v localhost:8080 2>&1|cat -v
[...]
* HTTP 1.0, assume close after body
< HTTP/1.0 200 OK
Everything is fine.
Have some tea.
[...]
$ echo -ne "HTTP/1.0 200 OK\r\nEvil: yes\033[2A a\r\n\r\nEverything is fine.\nHave some tea.\n" | nc -l -p 8080 &
$ curl -v localhost:8080 
[...]
* HTTP 1.0, assume close after body
< HTTP/1.0 200 OK^M
< Evil: yes^[[2A a^M
< ^M
[...]

Examples of the filter I talked about:

Obviously the filter only needs to be applied to the output that are passed from a remote.

It was vague, but not in the direction you were asking in. My goal is that terminal users don't need to know whether each binary correctly filters untrusted input when printing it to the terminal. Some of the regularly used binaries in an interactive shell will likely never be fixed. So I realise now I'll need something in the interactive shell to fix that anyway, even if curl were fixed. Thus I will not write a patch for curl for now.

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

Successfully merging a pull request may close this issue.

4 participants