DittoETH

Ditto
DeFiFoundryOracle
55,000 USDC
View results
Submission Details
Severity: medium
Invalid

Unfair distribution of fees on withdraw

Summary

Users are not obligated to withdraw their funds from the same bridge where they made their deposit. This can result in some users encountering higher withdrawal fees, which may seem unfair.

Vulnerability Detail

The BridgeRouterFacet is responsible for routing users' funds between different bridges, currently including BridgeReth and BridgeSteth. Users have the option to deposit LSTs, which can be in the form of rETH or stETH, or they can choose to depositETH, which will then be converted into LST. In return, users will receive zETH. Subsequently, users can withdraw their funds, a process that will remove their zETH from the corresponding vault and return the LST to them. Alternatively, they can choose to unstakeEth, which will unstake the zETH and send them the native ETH.

Both withdraw and unstakeETH may have different fees, depending on the bridge fees, and these fees will be charged to the user for these two operations.

As can be seen in the OwnerFacet.createBridge, bridges can be created with or without fees for withdraw/unstakeEth, with the only restrictions being an upper bound of 15% for withdraw and 2.5% for unstake. This means that one bridge can have fees while the other may not, or one bridge may have lower fees than the other.

Every user can deposit through the bridge of their choice, but they are not obliged to withdraw/unstakeEth from the same bridge.

Taking this into account, it may not be reasonable to withdraw/unstakeEth from the same bridge if the fees are higher. Consequently, this situation can lead to the funds from the bridge with lower or no fees being depleted first, forcing subsequent users to withdraw/unstakeEth from the other bridge with higher fees.

Proof Of Concept

In the provided test case, Alice and Bob deposited an equal amount of stETH and rETH in both of the bridges. However, only the BridgeReth imposes a withdrawal fee. As a result, when Alice decided to withdraw all of her tokens through the BridgeSteth (the bridge without a fee), Bob was compelled to use the other bridge, which incurs significant fees. This distribution of fees is unfair because Alice and Bob deposited an equal amount in both bridges, and therefore, the fees should also be shared during the withdrawal process.

Place the following test case in test/Bridges.t.sol and run it with the command forge test --mt test_unfair_fee_distribution -vvv.

pragma solidity 0.8.21;
import {OBFixture} from "test/utils/OBFixture.sol";
import {Vault, Constants} from "contracts/libraries/Constants.sol";
import {console} from "contracts/libraries/console.sol";
contract Bridges is OBFixture {
address alice = address(0x5000);
address bob = address(0x5001);
function setUp() public virtual override {
super.setUp();
vm.label(alice, "alice");
vm.label(bob, "bob");
}
function test_unfair_fee_distribution_simplified() external {
// set fees of the bridges
vm.startPrank(owner, owner);
diamond.setWithdrawalFee(_bridgeSteth, 0);
diamond.setUnstakeFee(_bridgeSteth, 0);
diamond.setWithdrawalFee(_bridgeReth, 1500);
diamond.setUnstakeFee(_bridgeReth, 250);
vm.stopPrank();
uint88 amount = Constants.MIN_DEPOSIT;
deal(_steth, alice, amount * 2);
deal(_reth, alice, amount * 2);
deal(_steth, bob, amount * 2);
deal(_reth, bob, amount * 2);
uint aliceTotalBalance = steth.balanceOf(alice) + reth.balanceOf(alice);
uint bobTotalBalance = steth.balanceOf(bob) + reth.balanceOf(bob);
console.log("Alice's balance before:", aliceTotalBalance);
console.log("Bob's balance before: ", bobTotalBalance);
assertTrue(aliceTotalBalance == amount * 4);
assertTrue(bobTotalBalance == amount * 4);
uint256 balanceBefore = diamond.getZethTotal(Vault.CARBON);
// Alice deposits 2e14 tokens in each of the bridges
vm.startPrank(alice);
steth.approve(_bridgeSteth, amount * 2);
diamond.deposit(_bridgeSteth, amount * 2);
reth.approve(_bridgeReth, amount * 2);
diamond.deposit(_bridgeReth, amount * 2);
vm.stopPrank();
// Bob deposits 2e14 tokens in each of the bridges
vm.startPrank(bob);
steth.approve(_bridgeSteth, amount * 2);
diamond.deposit(_bridgeSteth, amount * 2);
reth.approve(_bridgeReth, amount * 2);
diamond.deposit(_bridgeReth, amount * 2);
vm.stopPrank();
uint256 balanceAfter = diamond.getZethTotal(Vault.CARBON);
assertTrue(balanceBefore + amount * 8 == balanceAfter);
// Alice withdraws all of her amount from the bridgeSteth which has a 0 fee
vm.prank(alice);
diamond.withdraw(_bridgeSteth, amount * 4);
// Assert that the tokens left are only in the bridgeReth, which has a withdrawal fee
assertTrue(bridgeReth.getZethValue() == amount * 4);
assertTrue(bridgeSteth.getZethValue() == 0);
vm.expectRevert();
vm.prank(bob);
diamond.withdraw(_bridgeSteth, amount * 2);
// Bob is forced to withdraw from the bridge with a fee, although he deposited in both
vm.prank(bob);
diamond.withdraw(_bridgeReth, amount * 4);
aliceTotalBalance = steth.balanceOf(alice) + reth.balanceOf(alice);
bobTotalBalance = steth.balanceOf(bob) + reth.balanceOf(bob);
console.log("");
console.log("Alice's balance after: ", aliceTotalBalance);
console.log("Bob's balance after: ", bobTotalBalance);
}
}

Result:

Alice's balance before: 400000000000000
Bob's balance before: 400000000000000
Alice's balance after: 400000000000000
Bob's balance after: 340000000000000

Impact

Some users may be subject to higher fees.

Tool used

Manual Review

Recommendation

Ensure that the user can withdraw/unstakeEth only from the bridge in which it was deposited.
Alternatively, require that all of the bridges has equal fees.

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Admin Input/call validation
PTolev Submitter
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Admin Input/call validation

Support

FAQs

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