HexBrain

curl · Report #2384833

CVE-2024-2004: Usage of disabled protocol

Validstatus
lowseverity
33upvotes
27 Mar 2024disclosed

How it was found

Summary:

--proto in some circumstances ENABLES all protocols after being given -all, potentially leading to sending sensitive data over an unencrypted channel.

Steps To Reproduce:

curl -Ivs --proto -all,-http http://curl.se This command should result in curl: (1) Protocol "http" disabled but it actually succeeds.

Supporting Material/References:

The example command above performs a request to curl.se over http, despite being http support being disabled, both implicitly (-all) and explicitly (-http). It appears that a --proto string starting with -all and only ever removes protocols without adding them fails in this way. For example:

Example failure scenarios (allowing ALL protocols to go through):

--proto -all
--proto -all,-http
--proto -all,-ftp,-smtp,-pop3

Example correct scenarios (behaving as documented):

--proto -all,https
--proto -all,https,-http
--proto -all,-ftp,-smtp,-pop3,pop3s
--proto -http

The only failure scenarios I could find were those that first removed all protocols with -all, then (optionally) removed additional, individual protocols, and never added any. The documented behaviour for this would be to fail ALL curl transfers, so it shouldn't happen anywhere in production situations (since generally people want to do transfers with curl). However, I could still imagine it happening, such as when the list of allowed protocols is programmatically computed in a system where a particular situation should not allow any transfers. But this bug means that transfers would go ahead, anyway.

In a situation where something is relying on curl to block an unencrypted transfer, the consequences of this bug can be pretty severe (unencrypted data travelling over the Internet) and therefore High severity. On the other hand, IF this is the only situation where this bug occurs (where all protocols are supposed to be disabled), then I'm guessing this generally pretty uncommon and therefore Medium or even Low may be more appropriate. It's hard to know if anyone is relying on this.

I bisected the problem to this commit which was released in 7.85.0 and introduced string-based protocol selection:

e6f8445edef8e7996d1cfb141d6df184efef972c is the first bad commit
commit e6f8445edef8e7996d1cfb141d6df184efef972c
Author: Daniel Stenberg <[email protected]>
Date:   Mon Jun 13 09:30:45 2022 +0200

    setopt: add CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR

    ... as replacements for deprecated CURLOPT_PROTOCOLS and
    CURLOPT_REDIR_PROTOCOLS as these new ones do not risk running into the
    32 bit limit the old ones are facing.

    CURLINFO_PROTCOOL is now deprecated.

    The curl tool is updated to use the new options.

    Added test 1597 to verify the libcurl protocol parser.

    Closes #8992

I discovered it while replying to https://github.com/curl/curl/issues/12942

An example test case to show the problem is attached.

Impact

Data can be sent over an unencrypted channel because curl'ls mechanism to prevent it does not work.

This report was disclosed publicly on HackerOne and belongs to its author and the program. See the original