haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.62k stars 697 forks source link

`cabal install` with qualified component leads to `curl` exception #7815

Open andreasabel opened 3 years ago

andreasabel commented 3 years ago

cabal install with qualified component leads to curl exception:

$ cabal install cabal-plan:exe:cabal-plan
Downloading cabal-plan:exe:cabal-plan
Error: cabal: '/usr/local/opt/curl/bin/curl' exited with an error:
curl: (3) URL using bad/illegal format or missing URL

Correct error given in 3.2:

$ cabal-3.2 install cabal-plan:exe:cabal-plan
cabal-3.2: Invalid package ID: cabal-plan:exe:cabal-plan

(I wonder whether this could be exploited: injecting code into the curl request. In any case, sanitization seems to be missing.)

Mikolaj commented 3 years ago

I can't repro on master. It compiles and installs fine. Your result is from 3.4.1? How about 3.6.2?

phadej commented 3 years ago

With

% cabal --version
cabal-install version 3.6.2.0
compiled using version 3.6.2.0 of the Cabal library 

Fails

% cabal install cabal-plan:exe:cabal-plan
Downloading cabal-plan:exe:cabal-plan
cabal: '/usr/bin/curl' exited with an error:
curl: (3) Port number ended with 'e'

because cabal-plan:exe:cabal-plan doesn't look like package id or pkg-id:component-name (see https://github.com/haskell/cabal/blob/master/cabal-install/src/Distribution/Client/CmdInstall/ClientInstallTargetSelector.hs)

but cabal-plan:exe:cabal-plan is valid URI:

Prelude> :m +Network.URI Data.Generics.Text
Prelude Network.URI Data.Generics.Text> gshow $ parseURI "cabal-plan:exe:cabal-plan"
"(Just (URI \"cabal-plan:\" (Nothing) \"exe:cabal-plan\" \"\" \"\"))"

cabal-install-3.4 got functionality to install from URI: https://github.com/haskell/cabal/pull/6576

I don't think this can be exploited. Of course user can run cabal install https://download/something/bad, but I don't know what's then? (malicious TH or Setup.hs code? that exploit is always available atm).

I didn't want to hardcode known schemes for URL download, as curl / wget etc know plenty. Maybe there could be some output of how the target selectors were elaborated (e.g. Installing from URI cabal-plan:exe:cabal-plan or Installing package id: cabal-plan or ...) and warned if the URI scheme is not http or https, but overall I think cabal works correctly.

Mikolaj commented 1 year ago

It's not entirely clear what the right behaviour should be here, so downgrading priority.

andreasabel commented 1 year ago

@phadej wrote:

I don't think this can be exploited. Of course user can run cabal install https://download/something/bad, but I don't know what's then? (malicious TH or Setup.hs code? that exploit is always available atm).

I do think so. Say you are reviewing not a cabal developer but review a PR that changes (somewhere in a big PR)

cabal install cabal-plan:cabal-plan

into

cabal install cabal-plan:exe:cabal-plan

with some comment that "the new, fully qualified package selector are used (more robust)". This guy has a malicious version of cabal-plan behind this URI. Joe Reviewer would likely not notice that we switch here from a local install to a remote install. Baammm! Door open.

A tool like cabal shouldn't lend itself to obfuscation tricks like that.

@Mikolaj wrote:

It's not entirely clear what the right behaviour should be here, so downgrading priority.

Well, the right behavior is:

  1. Have means to explicitly disambiguate package id and package component targets from URI targets, like --id and --uri. (Don't we all love types?)
  2. Without explicit disambiguation, reject ambiguous cases, meaning targets that parse as package components and URIs. (Don't we all like type inference to not make arbitrary choices?)

The current code just tries parsing package component first and URI later, and does not care about the overlap: https://github.com/haskell/cabal/blob/cb2b639efd8c2abdfbc0127d0ad9565ab6d7a878/cabal-install/src/Distribution/Client/CmdInstall/ClientInstallTargetSelector.hs?rgh-link-date=2021-11-13T14%3A43%3A51Z#L23-L43

@phadej wrote:

but cabal-plan:exe:cabal-plan is valid URI

cabal-plan:cabal-plan is also a valid URI but the current strategy will never admit it as URI target.