Shellcheck: Check: curl/wget piped to sh

[scovetta]

For new checks and feature suggestions

• [X] shellcheck.net (i.e. the latest commit) currently gives no useful warnings about this
• [X] I searched through https://github.com/koalaman/shellcheck/issues and didn't find anything related

Here's a snippet or screenshot that shows the problem:

#!/bin/bash

curl https://dodgy-website/install.sh | sh
curl https://dodgy-website/install.sh | sudo foo

Here's what shellcheck currently says:

(Nothing)

Here's what I wanted or expected to see:

In file.sh:
curl https://dodgy-website/install.sh | sh
^--^ SC0000: Ensure that only trusted scripts are downloaded and executed.

curl https://dodgy-website/install.sh | sudo foo
^--^ SC0000: Ensure that only trusted scripts are downloaded and passed to `sudo`.

Basically, there's a common, but dangerous practice of downloading scripts and immediately passing them to another shell (sh, bash, etc., but it could be any interpreter). Downloading is most commonly done using curl or wget.

Is this the type of rule that belongs in Shellcheck? My Haskell is very, very beginner, but I'd be happy to submit a PR for feedback if you're interested.

[gromgit]

I like the premise of this suggestion, but I don't think it goes far enough. _All_ shell invocations that don't specify a script file or a command (via -c) should be warned about, as they would be taking their logic from a source that cannot be immediately scrutinized. curl and wget are merely a subset of the dangerous possibilities.

Also:

curl https://dodgy-website/install.sh | sudo foo

That tells sudo to execute the command foo with stdin fed from curl, which is a common paradigm for many config steps, e.g.

# dynamically install repository public keys
curl https://some.place.net/pub.asc | sudo apt-key add -

I assume you mean:

curl https://dodgy-website/install.sh | sudo -i  # or "sudo -s"

which actually runs curl's output in a root shell.

[brother]

As much as I have been in the same camp as where this suggestions stems from I am more on the line of what is written in this blog post these days.
https://www.arp242.net/curl-to-sh.html

I am not certain that it is up to shellcheck to make claims in this area.

[rpdelaney]

curl | sh is a somewhat lazy distribution practice with real drawbacks. For instance, a linter that we want to run in CI offered this method for installation. It's important that such a tool running in CI be pinned to a version so that engineers don't get unexpected failures in CI after the code passes in their local environment. Since curl | sh inhibits version pinning, I had to engineer my own workaround for CI's pre-test setup process.

It _is_ also less secure, as that blog post itself states:

Package managers are more secure due to checksums, signing, and auditing.

Of course it's not some kind of security armageddon. But where to draw the line is a decision.

The suggested language in this feature request is not unreasonable imo. I don't know what's wrong with providing a warning. A workflow with a pre-commit hook for shellcheck would mean one must explicitly disable this warning as a way of asserting that yes, I have vetted this source and decided they are trustworthy. That's something people should be doing when they are executing someone else's code dynamically in a manner that is flatly incompatible with version pinning or auditing.

[bureado]

which actually runs curl's output in a root shell.

While curl | sh is a category of its own, I think @scovetta also reflects on things like curl | sudo tee that basically mutate the system in a blunt way. The warning could point at that to ensure the script author understands that risk. In other words, a curl | sudo apt-key has recognizable intent (even though it could expose the system) whereas a curl | sudo tee can wreak havoc in much more generic ways.

[bureado]

I am not certain that it is up to shellcheck to make claims in this area.

This author's response to the "isn't so bad" article is that publishers start optimizing for the kind of user that is otherwise well served by curl | sh, with installers that want to dictate and end up taking over the client experience. I don't think script authors would expect shellcheck to say: "curl | sh detected, use your package manager instead", but I also think a warning could save time and resources compared to a runtime sandbox approach. Here's a more complete list of vectors, and an exploration of the server-side vector.

[scovetta]

Thanks for the feedback, everyone!

Perhaps labeling this check as info rather than warn? As an end-user, if I curl | sh or even curl | sudo sh, I'm at least marginally aware that I'm doing something dangerous, but I'd have no clue if a package that I added was doing this as install/run-time.

If you agree, I'd suggest we be pragmatic, and not try to solve the general class initially -- but maybe it's larger than [curl|wget] | sudo? [-i|bash|sh].

[TinCanTech]

Please to look at this logical eye:

Assume the discussion has run it's course and it is decided that shellcheck _should_ emit a warning regarding pipe to sudo (or similar).

Would shellcheck then include a #shellcheck disable=pipe2sudo type directive ? Would not the developer then choose to use such a directive ?

And then, if _another_ user chooses to shellcheck a script from the _interweebs_ what do they do ? Any shellcheck check can be suppressed.

1417