Blockchain bridge security - Part 4
This is the 4th article of the Bridge security series. see the Part 1, Part 2, part 3.
This article explains:
- Chain id spoofing
- Hash collision
- Lack of signature expiry check
1. Chain id spoofing
The contract used in this example is BridgeSpoofChainId.sol. This contract has one additional function named setAllowedSrcChain(), which allows setting chain ids (actually, it maps the boolean value to the number) which then is used in the executeMessage() in the require statement to check the entered transaction.srcChainId is allowed.
While there's no actual need of this require statement in this example. it is added in this example to explain how the chain id can be spoofed.
The test test_chain_id_validation() shows that the transaction reverts when srcChainId is set to the chain id number that is not allowed by the contract, In this case the transaction failure is expected.
test_chain_id_spoofing() shows chain id spoofing. Where, just for the example, owner sets the 56 to true with destBridge.setAllowedSrcChain().
Then the transaction struct is created. As can be seen in the data field that the transaction is for transferring 1e18 ERC20 tokens to user on destination chain. Interestingly the transaction.srcChainId is set to 56.
Then it signs and sends the transaction on source chain bridge with bridge.sendMsgPermit()
Then it executes destBridge.executeMessage(). Since on BridgeSpoofChainId.sol#L147-L148 both the if{} would be true, it executes internal _transfer() to transfer the decoded value (that is 1e18), to the decoded to address (that is user address) from the transaction.from.
The transaction executes successfully. And on these lines in the test, it can be seen that the transaction is successfully processed, The user address received 1e18 tokens from the Alice (on destination chain) and the Alice's balance becomes 0.
So there can be different possibilities here, e.g. The destination bridge (for the sake of this example) does not support the current chain id the user is on, but supports chain id 56. In this case, the user did not have enough tokens to transfer from the chain id 56. So he spoofs the chain id, sets it to 56 (even though he is not on chain id 56). and because the sendMsgPermit() (or sendMsg()) fails to set transaction.srcChainId = block.chainid the user is able trick the logic and the transaction is successful.
As you might have noticed we don't need this require check to show this vulnerability, but it was added to the example to show how chain spoofing was possible; tricking the chain id from current source chain.
2. Hash collision
test_sendMsg_hash_collision() shows the hash collision in the logic and how it affects the working of the contract. The contract used in this example is BridgeHashCollision.sol
The test uses for{} loop to iterate 12 times. As can be seen for every iteration it sets the value according to i. If i is 12 then for that iteration it sets the value as 20000000000000000 (0.02e18). Otherwise the value is always 120000000000000000 (0.12e18).
Then it creates the transaction struct for transferring the value of ether to the user address on the destination chain. then it sends the message with bridge.sendMsg().
Then using vm.chainId(2), it sets the chain id to 2. why? because the destination chain id (dstChainId) set in the transaction struct is 2, so while calling destBridge.executeMessage() if the block.chainid is not 2, the transaction will revert because of the the validation.
Finally it calls destBridge.executeMessage() to execute the transaction on the destination chain.
Lastly, the chain id is set back to transaction.srcChainId on BridgeHashCollision.t.sol#L61, the reason behind it is, On the BridgeHashCollision.t.sol#L45 it was set to 2 for executing destBridge.executeMessage() successfully as explained above, Now because all the function calls are working in the loop, it's important to set the chain id back again to what it was. transaction.srcChainId was set to the original chain id while creating the struct, so we use that value to set the chain id back to what it was on BridgeHashCollision.t.sol#L61.
Understanding the actual problem caused by hash collision:
In BridgeHashCollision.sol while every time calculating the messageHash it needs to encode all the variables (function arguments) before hashing them. interestingly it uses the abi.encodePacked for encoding.
Now the first impression here is, because sendMsg() and sendMsgPermit() always increments messageId (which is converted to idString, which is used while finding the messageHash) it should protect against the hash collision. But this assumption is wrong.
On BridgeHashCollision.sol#L115 in messageHash calculation the hash collision can happen. In this example the collision happens because we are keeping most of the values in transaction struct same as before. Even though we assume the unique id protects against the collision, what works against this assumption is the usage of abi.encodePacked.
If we check the test again it expects the 12th iteration of for{} loop to revert with "Bridge: message already processed" on BridgeHashCollision.t.sol#L50 i.e. it expects the the transaction to be already processed (because of hash collision).
This hash collision happens because of non standard encoding is used to create messageHash.
So in this example used in the test, the encoding for the data when the id is 1 and 11 would be exactly same. as can be seen below, for the id 1 the value used is 120000000000000000 (0.12e18) and for the id 11 the value used is 20000000000000000 (0.02e18). So eventually the non padded encoding (using abi.encodePacked) gives the same result. and because the other variable passed are the same for all the iterations in our test. the messageHash is same. So executing executeMessage() for the same messageHash would revert because the bridge assumes the transaction has already executed.
Below is the diagram showing how the collision looks like in messageHash.
In current example the problem it creates is Denial of Service (DoS), but this problem can cause any other critical vulnerability. Since, the bridge implementations might many time consist of multiple variables in the message/struct that's getting forwarded and can consist the use of hashing (because of storing the unique proofs of bridges etc.). It becomes crucial to check if the bridge logic is vulnerable to these type of scenarios and problem it creates.
3. Lack of signature expiry check
One more important thing to take care while dealing with signatures. It is good idea to use expiry timestamp in struct/data for which the signer signs the signature.
Since, the expiry would be the part of the the struct/data, it can be checked in the function which verifies the signatures. On the code level it can be checked that if the current timestamp i.e. block.timestamp should be less than (or equal to according to the requirement) the expiry timestamp mentioned in the date/struct.
This will ensure that if the signature doesn't get executed for a while or for specific amount of time, it will be later treated as an invalid signature. And the simple reason behind this can be any situation where signer is not willing to allow the usage of that signature. e.g in this case if signer signs the data for transferring certain amount of tokens, maybe that signer won't be comfortable executing that signature after 6 months or a year because of many possible reasons.
It is important to give a thought that there can be dedicated relayers for executing/submitting these signed signatures. Some projects might not need the signature expiry checks because they are sure that the signature would be surely executed in the certain amount of time.
Apart from the common vulnerabilities discussed in this series, there can be additional and unique vulnerabilities depending on the smart contract and business logic.
