Submission Details
Severity:
During withdrawal tokens can be set to arbitrary address
[H-3] During withdrawal tokens can be set to arbitrary address
Description:
The withdrawTokensToL1 method allows specifying the address to be withdrawn to. So an arbitrary user could be receiver of the tokens in the vault
Impact:
High
Tools used:
foundry
Proof of Concept:
function testIfSignerCanWithdrawTokensToArbitraryAddress() public {
// user deposits 10 tokens to Vault
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(vault)), depositAmount);
assertEq(token.balanceOf(address(user)), userInitialBalance - depositAmount);
vm.stopPrank();
// operator withdraws 10 tokens from Vault to arbitrary address
address addr = makeAddr("randomAddress");
(uint8 v, bytes32 r, bytes32 s) = _signMessage(_getTokenWithdrawalMessage(addr, depositAmount), operator.key);
tokenBridge.withdrawTokensToL1(addr, depositAmount, v,r,s);
assertEq(token.balanceOf(addr), depositAmount);
// Mitigate by holding a mapping of user => amount deposited
}
Recommended Mitigation:
Use mapping of user => amount_deposited and when withdrawing check if amount is not greater than what is the amount in the mapping.
diff --git a/src/L1BossBridge.sol b/src/L1BossBridge.sol
index 8fddb07..3f73626 100644
--- a/src/L1BossBridge.sol
+++ b/src/L1BossBridge.sol
import { L1Vault } from "./L1Vault.sol";
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
using SafeERC20 for IERC20;
uint256 public DEPOSIT_LIMIT = 100_000 ether;
IERC20 public immutable token;
L1Vault public immutable vault;
mapping(address account => bool isSigner) public signers;
+ mapping(address account => uint256 amount) public amountDeposited;
error L1BossBridge__DepositLimitReached();
error L1BossBridge__Unauthorized();
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
_unpause();
}
function setSigner(address account, bool enabled) external onlyOwner {
signers[account] = enabled;
}
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
if (token.balanceOf(address(vault)) + amount > DEPOSIT_LIMIT) {
revert L1BossBridge__DepositLimitReached();
}
+ amountDeposited[from] += amount;
token.safeTransferFrom(from, address(vault), amount);
// Our off-chain service picks up this event and mints the corresponding tokens on L2
contract L1BossBridge is Ownable, Pausable, ReentrancyGuard {
* @param r The r value of the signature
* @param s The s value of the signature
*/
function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ require(amount <= amountDeposited[to], "L1BossBridge: amount is less than deposited");
+ amountDeposited[to] -= amount;
sendToL1(
v,
r,
s,
abi.encode(
address(token),
0, // value
abi.encodeCall(IERC20.transferFrom, (address(vault), to, amount))
)
);
}
Updates
Lead Judging Commences
0xnevi over 1 year ago
Submission Judgement Published Assigned finding tags:
withdrawTokensToL1(): No check for deposits amount
hueber
over 1 year ago
0xnevi
over 1 year ago
0xnevi over 1 year ago
Submission Judgement Published Assigned finding tags:
withdrawTokensToL1(): No check for deposits amount
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.