Skip to content

cabal install with qualified component leads to curl exception #7815

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

Open
andreasabel opened this issue Nov 13, 2021 · 4 comments
Open

cabal install with qualified component leads to curl exception #7815

andreasabel opened this issue Nov 13, 2021 · 4 comments

Comments

@andreasabel
Copy link
Member

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
Copy link
Member

Mikolaj commented Nov 13, 2021

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

@phadej
Copy link
Collaborator

phadej commented Nov 13, 2021

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: #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
Copy link
Member

Mikolaj commented Dec 15, 2022

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

@andreasabel
Copy link
Member Author

@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:

data WithoutProjectTargetSelector
= WoPackageId PackageId
| WoPackageComponent PackageId ComponentName
| WoURI URI
deriving (Show)
parseWithoutProjectTargetSelector :: Verbosity -> String -> IO WithoutProjectTargetSelector
parseWithoutProjectTargetSelector verbosity input =
case explicitEitherParsec parser input of
Right ts -> return ts
Left err -> case parseURI input of
Just uri -> return (WoURI uri)
Nothing -> die' verbosity $ "Invalid package ID: " ++ input ++ "\n" ++ err
where
parser :: CabalParsing m => m WithoutProjectTargetSelector
parser = do
pid <- parsec
cn <- optional (char ':' *> parsec)
return $ case cn of
Nothing -> WoPackageId pid
Just cn' -> WoPackageComponent pid (CExeName cn')

@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.

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

No branches or pull requests

4 participants