Closed c4-bot-3 closed 8 months ago
DadeKuma marked the issue as insufficient quality report
QA might be more appropriate. Docs explicitly say to use p2wpkh
.
0xean marked the issue as unsatisfactory: Overinflated severity
@DadeKuma @0xean Hi, thanks for the review and comments. Please note the doc and code explicitly say to use p2wpkh
, but that is for the Bitcoin tx's second output, not for the inputs (i.e. the UTXO belongs to users) used. So users are free to use any UTXO, which is likely to happen as shown in the description, and can lead to users' assets lost. Since it's triggered by the users themself, Medium might be right.
I think this is mainly an issue with documentation and certainly think H is overinflated. I will ask for sponsor comment before making a final decision
thanks for the report this is a valid issue; we will update the doc so that it's clear only p2wpkh inputs/outputs are supported.
0xean changed the severity to QA (Quality Assurance)
0xean marked the issue as grade-b
Lines of code
https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/node/zetaclient/bitcoin_client.go#L582 https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/node/zetaclient/bitcoin_client.go#L667
Vulnerability details
Impact
On Bitcoin chain, If users send assets to the TSS address with the first tx.input UTXO types other than p2wpkh (i.e. don't have witness or the second field is not public key), zetachain client will ignore their sends or fail to refund the assets, leading to losses. Please note that this is not a user error and likely to happen, because:
p2wpkh to TSS
+OP_RETURN PUSH_x [DATA]
, not the inputs (UTXO) they use.Proof of Concept
We filter and parse send events from btc in
zetaclient/bitcoin_client.go
:As we can see, if we fail to get the btc event in
GetBtcEvent
, we will ignore this transaction and continue to handle next one. Even more, after we get all the usable events, the last scanned block will be set, so the failed get events won't be handled in the future:The problem is in
GetBtcEvent()
: As show below, after we get the target address and calling parameters (memo
), we need to get theFromAddress
which will be used as thesender
,txOrigin
andreceiver
inGetInBoundVoteMessage
to send to the zetacore:To get the
FromAddress
, we hash the second witness field (public key if the corresponding output in UTXO is p2wpkh) in the first input to an address, which is error-prone:tx.Vin
. In reality, it depends on the user's bitcoin wallet to send the transaction. If it doesn't meetif len(vin.Witness) == 2
,fromAddress
will be unassigned "", this will make zetachain unable to refund the assets if the call on zEVM revert.In these cases, we will failed to decode/convert the second field to an address and return an error, making the transaction be ignored.
Tools Used
Manual Review.
Recommended Mitigation Steps
fromAddress
is not empty.Assessed type
Context