search

summa-tx / bitcoin-spv

utilities for Bitcoin SPV proof verification on (many!) other chains
Other
170 stars 45 forks source link

extractHash does not check the size of the script #161

Open sr-gi opened 4 years ago

sr-gi commented 4 years ago

Description

extractHash checks that the template for P2PKH, P2SH, P2PKH and P2WSH is valid (proper prefix and suffix), but the actual size of the provided script is never checked.

https://github.com/summa-tx/bitcoin-spv/blob/3a35db8480110970775abb8f6f89956f7872d35f/solidity/contracts/BTCUtils.sol#L406-L434

Therefore a script encoded under the following format will be accepted:

P2PKH: 1976a914 <script_with_more_than_20_bytes> 88ac
P2SH: 17a914 <script_with_more_than_20_bytes> 87
P2PKH: 0014 <script_with_more_than_20_bytes>
P2PKH: 0020 <script_with_more_than_32_bytes>

This can cause issues if the provided data does not come from a validated transaction, for instance if the data is being provided directly from a user.

This is related to https://github.com/keep-network/tbtc/issues/658

Invalid P2PKH example

The following call won't be caught, even though the script is invalid (notice the extra 88 by the end of the script):

extractHash(0x4062b007000000001976a914f86f0bc0a2232970ccdf4569815db500f12683618888ac)

Fix

Add an additional check for maliciously formatted scripts to check the actual length of the provided script.

sr-gi commented 4 years ago

Update: this actually affects both segwit and legacy outputs.

prestwich commented 4 years ago

Related: #155 #156

As a general rule, the parser does not handle invalid structures. Inputs to this function are assumed to have passed validateVout and some inclusion proof, so validity checks wrt length are not necessary here