Skip to content

os/exec: document lack of shell expansion more #20894

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

Closed
bradfitz opened this issue Jul 3, 2017 · 8 comments
Closed

os/exec: document lack of shell expansion more #20894

bradfitz opened this issue Jul 3, 2017 · 8 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jul 3, 2017

It's becoming a FAQ that people expect os/exec to expand '*' etc like the shell. (#20893 and a number more)

Many scripting languages provide such APIs (often by default), even though they're easy to misuse security-wise.

Let's document that os/exec requires explicit shell invocation or explicit globbing.

@bradfitz bradfitz added the Documentation Issues describing a change to documentation. label Jul 3, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone Jul 3, 2017
@davidrenne
Copy link

Thanks Brad!

@davidrenne
Copy link

Brad my one comment would be that maybe if it's possible to return a parse error when splat or similar are passed. Many people might miss the new documentation and it'd be great to see a valid error rather than error code 2. But I realize that star can be used in a parameter and still be valid so it might be difficult to support generically with better error messages.

No biggie but there's my two cents. Lol. I wrote a file to the system with the splats and called it from golang as a workaround.

@bradfitz
Copy link
Contributor Author

bradfitz commented Jul 3, 2017

We don't parse.

@cznic
Copy link
Contributor

cznic commented Jul 3, 2017

Having a cross platform version of system(3) in os[/exec] might be an answer as well.

@robpike
Copy link
Contributor

robpike commented Jul 3, 2017

No! Do not put system(3) in the library. It's one of the most glaring security holes a library can provide.

Instead, document in os/exec that you get what you say and if you need the shell, use it - carefully!

@cznic
Copy link
Contributor

cznic commented Jul 4, 2017

One can already do exec.Command("/bin/sh", "-c", whatever) today. Is there something different from the security POV wrt a hypothetical exec.System(whatever)?

@robpike
Copy link
Contributor

robpike commented Jul 4, 2017

Yes. You have to be knowledgeable enough to know about sh -c.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/47550 mentions this issue.

@golang golang locked and limited conversation to collaborators Jul 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

5 participants