UniValue upgrade needed to avoid UniValue CVE-2019-18936 (segfault via high object depth JSON)

practicalswift commented at 10:33 am on December 13, 2019: contributor

While auditing the Bitcoin Core RPC code I found an issue in UniValue. UniValue::read() in UniValue 1.0.3 and 1.0.4 allows a segfault via high object depth JSON.

Live demo:

0$ src/bitcoind -regtest -rpcuser=u -rpcpassword=p > /dev/null &
1[1] 8625
2$ curl -u u:p --request POST \
3      --data $(python -c 'print(50000 * "[");') http://127.0.0.1:18443
4curl: (52) Empty reply from server
5[1]+  Segmentation fault      (core dumped) src/bitcoind -regtest -rpcuser=u -rpcpassword=p > /dev/null

I reported the issue to security@ and UniValue author @jgarzik back in early November 2019. The issue was fixed in commit https://github.com/jgarzik/univalue/commit/fe2227d4ace9d4d5b30710684c9d3b1012e3c0fc (https://github.com/jgarzik/univalue/pull/64). The issue in UniValue has been assigned CVE-2019-18936.

We need to bump our UniValue version to address this issue :)

CVE details submitted to MITRE by @jgarzik:

Suggested description: UniValue::read() in UniValue 1.0.3 and 1.0.4 allows a segfault via malformed JSON.
Additional Information: UniValue is a JSON I/O library commonly used in cryptocurrency applications.
VulnerabilityType Other: Denial of Service
Vendor of Product: univalue
Affected Product Code Base: univalue - 1.0.4, univalue - 1.0.3
Affected Component: UniValue::read() method
Attack Type: Context-dependent
Impact Denial of Service: true
Attack Vectors: Supply malformed JSON into UniValue’s JSON parser.
Reference: https://github.com/jgarzik/univalue
Has vendor confirmed or acknowledged the vulnerability?: true
Discoverer: practicalswift
Reference: https://github.com/jgarzik/univalue/compare/v1.0.4...v1.0.5, https://github.com/jgarzik/univalue/pull/58

practicalswift added the label Bug on Dec 13, 2019

fanquake added the label Utils/log/libs on Dec 13, 2019

MarcoFalke commented at 4:45 pm on December 13, 2019: member

As far as we are concerned, this is just a stylistic issue. Even with the fix, anyone with access to the rpc interface can still shut down the node (either by directly calling stop or #15017 or #14376 or …)

practicalswift commented at 10:27 pm on December 13, 2019: contributor

@MarcoFalke

As far as we are concerned, this is just a stylistic issue.

It would be silly for me to try to oversell this relatively small issue, but calling it “just a stylistic issue” from a Bitcoin Core perspective does not feel like an accurate description TBH :)

Point 1. RPC user whitelisting

With the merge of RPC user whitelisting (PR #12763) we now effectively have “not-as-trusted-as-the-node-operator” RPC consumers whether we like it or not. Any such not fully trusted RPC consumer can trivially trigger this UniValue bug as demonstrated by the curl example above and shut down the node that he or she has been given supposedly locked down RPC access to.

I don’t think that is behaviour that our users expect.

Point 2. Partial control over JSON payload when Bitcoin Core is used as a service backend

It sounds like you’re assuming that access to the RPC interface is either full access (full control over the entire JSON payload sent to the node) or no access (no control over the JSON payload).

From a “bitcoin-cli perspective” it is easy to get the impression that access is always binary and that the node operator is the only person being able to control the JSON payload.

In practice the RPC is interface is often used as a backend to various services (such as web interfaces, etc.). By interacting with these services an attacker can influence gain partial control over the JSON payload sent to the node.

This is a typical Bitcoin Core RPC JSON payload:

0{
1   "id" : 1,
2   "params" : [… some object structure …],
3   "method" : "method_to_call"
4}

In a typical RPC-used-from-a-web-application the attacker is probably not being able to influence which method to call ("method" : "method_to_call" in the example above), but it is not unreasonable to assume that he/she may be able to influence at least a subset of the object structure that is sent as parameters to the RPC method ("params" : [… some object structure …]) by legitimate interaction with the service.

So while an attacker is likely not be able to change the “method” field from say help to stop it is not unthinkable that he or she might be able to cause some part of the params structure to be of excessively high object depth and thus triggering this UniValue issue and shuts down the backend Bitcoin Core node.

And even in the case that such high object depth would be due to lacking validation on the part of the application developer I think the expectation of our users would be that Bitcoin Core should return a parsing error and not die as it currently does :)

Given this clarification, can you see why calling this “just a stylistic issue” does not feel entirely accurate? :)

practicalswift commented at 5:28 pm on December 14, 2019: contributor

Demonstration of a locked down RPC user with supposedly only access to the help command (-rpcwhitelist=u:help) segfaulting the node via a single curl command:

 0$ share/rpcauth/rpcauth.py u p
 1String to be appended to bitcoin.conf:
 2rpcauth=u:d7ca300b5aeceb73e9825068fc4496ef$166cbdedfe11c30a95c28d13b3383682825308d1069d347f9705b8cf922637c5
 3Your password:
 4p
 5$ src/bitcoind -regtest -rpcwhitelist=u:help '-rpcauth=u:d7ca300b5aeceb73e9825068fc4496ef$166cbdedfe11c30a95c28d13b3383682825308d1069d347f9705b8cf922637c5' &
 6$ src/bitcoin-cli -regtest -rpcuser=u -rpcpassword=p help | head -1
 7== Blockchain ==
 8$ src/bitcoin-cli -regtest -rpcuser=u -rpcpassword=p getnetworkinfo
 92019-12-13T02:01:37Z RPC User u not allowed to call method getnetworkinfo
10error: server returned HTTP error 403
11$ curl -u u:p --request POST \
12     --data $(python -c 'print(50000 * "[");') http://127.0.0.1:18443
13curl: (52) Empty reply from server
14[1]+  Segmentation fault      (core dumped) src/bitcoind -regtest -rpcwhitelist=u:help '-rpcauth=u:d7ca300b5aeceb73e9825068fc4496ef$166cbdedfe11c30a95c28d13b3383682825308d1069d347f9705b8cf922637c5'

Am I the only one thinking that this might not be the behaviour our users expect when using -rpcwhitelist, and thus not just being a “stylistic issue”? :)

practicalswift renamed this:
~~UniValue upgrade needed to avoid UniValue CVE-2019-18936 (segfault via malformed JSON)~~
UniValue upgrade needed to avoid UniValue CVE-2019-18936 (segfault via high object depth JSON)
on Dec 14, 2019

laanwj commented at 10:07 am on December 15, 2019: member

It’s a bug and should be fixed like any other…

So to be concrete and make progress here:

The fix needs to end up in our univalue repo first (filed a PR bitcoin-core/univalue#22)
Then a subtree pull from there can be done into this repo
Then this can be PRed here

laanwj added this to the milestone 0.20.0 on Dec 15, 2019

Sjors commented at 11:26 am on January 4, 2020: member

The UniValue PR has been merged, so subtree update is the next step.

MarcoFalke commented at 1:57 pm on January 4, 2020: member

As we bump anyway, we might as well merge those stylistic changes from upstream: https://github.com/bitcoin-core/univalue/pull/23

fanquake closed this on Feb 10, 2020

ftrader referenced this in commit d2a20cc133 on Apr 16, 2020

PastaPastaPasta referenced this in commit 52c55d63eb on Jul 17, 2021

DrahtBot locked this on Feb 15, 2022

UniValue upgrade needed to avoid UniValue CVE-2019-18936 (segfault via high object depth JSON) #17742