elements: supplying invalid blinding factors to rawblindrawtransaction causes node to crash
if invalid value blinder is supplied, this assert is triggered:
if invalid asset blinder is supplied, this CHECK() in secp256k1 is triggered:
Triggered with this small patch to qa/rpc-tests/confidential_transactions.py (tested on 0.14.1 branch, but the failing code is the same on master. to trigger first case, you just put ab in place of value blinder, not asset blinder as in this patch):
diff --git a/qa/rpc-tests/confidential_transactions.py b/qa/rpc-tests/confidential_transactions.py
index 61393fc6e..93c7b5cc5 100755
--- a/qa/rpc-tests/confidential_transactions.py
+++ b/qa/rpc-tests/confidential_transactions.py
@@ -470,6 +470,12 @@ class CTTest (BitcoinTestFramework):
except JSONRPCException:
pass
+ try:
+ ab = 'FF'*32
+ blindtx = self.nodes[0].rawblindrawtransaction(rawtx, [unspent[0]["blinder"], unspent[1]["blinder"]], [unspent[0]["amount"], unspent[1]["amount"]], [unspent[0]["asset"], unspent[1]["asset"]], [unspent[0]["assetblinder"], ab])
+ except JSONRPCException as e:
+ print(e)
+
blindtx = self.nodes[0].rawblindrawtransaction(rawtx, [unspent[0]["blinder"], unspent[1]["blinder"]], [unspent[0]["amount"], unspent[1]["amount"]], [unspent[0]["asset"], unspent[1]["asset"]], [unspent[0]["assetblinder"], unspent[1]["assetblinder"]])
signtx = self.nodes[0].signrawtransaction(blindtx)
txid = self.nodes[0].sendrawtransaction(signtx["hex"])
While the impact of the bug is not very serious, as this can only be triggered with authenticated rpc request and results only in node crash, I believe the crash is not correct response to invalid user input.
First case can be fixed by checking ret and returning failure, instead of dying on assert.
But the second case is not that easy, and would require checking that a given blinder represents valid scalar before passing it to secp256k1_generator_generate_blinded. I did not find appropriate function in secp256k1 that would allow to check this condition before passing the data to secp256k1_generator_generate_blinded.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 16 (10 by maintainers)
Commits related to this issue
- Merge #653: Update secp-zkp to 44db4d801fff3cd94105136cb443d603683baad2 c866f52e2 Squashed 'src/secp256k1/' changes from 43dd1f4..44db4d8 (Gregory Sanders) Pull request description: Resolves http... — committed to ElementsProject/elements by stevenroose 5 years ago
- Merge #661: Don't assert on user-inputed values f9d159b Don't assert on user-inputed values (Steven Roose) Pull request description: This prevents the assertion from crashing the node when an RPC... — committed to ElementsProject/elements by instagibbs 5 years ago
Yeah
UnblindConfidentialPairis the only one without asserts 😃So #661 covers those two asserts and those are indeed the ones hit when providing incorrect blinders.
The blind.cpp file is full of asserts, most of them seem to be from self-generated information, though. Luckily that file is only exposed to RPC users.