elements: supplying invalid blinding factors to rawblindrawtransaction causes node to crash

if invalid value blinder is supplied, this assert is triggered:

https://github.com/ElementsProject/elements/blob/dd1623af0a4b028175e1078f7ec7a88d78cee8c0/src/blind.cpp#L517

if invalid asset blinder is supplied, this CHECK() in secp256k1 is triggered:

https://github.com/ElementsProject/elements/blob/dd1623af0a4b028175e1078f7ec7a88d78cee8c0/src/secp256k1/src/modules/generator/main_impl.h#L178

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

Most upvoted comments

Yeah UnblindConfidentialPair is 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.