DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

User lose any funds deposited via `BridgeRouterFacet.sol`

836541

Summary

In BridgeRouterFacet.sol, user has two methods to deposit collateral in the protocol: deposit and depositEth. Both call vault.addZeth() to increase user's balance, which is a function that increases the balance of msg.sender. However, msg.sender is the BridgeRouterFacet.sol, not the user itself, so only the Bridge balance increases.

Vulnerability Details

  1. User want to deposit ETH, stETH or rETH to get balance in the protocol. He calls deposit or depositEth.

  • deposit:

function deposit(address bridge, uint88 amount)
external
nonReentrant
onlyValidBridge(bridge)
{
if (amount < Constants.MIN_DEPOSIT) revert Errors.UnderMinimumDeposit();
// @dev amount after deposit might be less, if bridge takes a fee
uint88 zethAmount = uint88(IBridge(bridge).deposit(msg.sender, amount)); // @dev(safe-cast)
​
uint256 vault;
if (bridge == rethBridge || bridge == stethBridge) {
vault = Vault.CARBON;
} else {
vault = s.bridge[bridge].vault;
}
​
vault.addZeth(zethAmount);
maybeUpdateYield(vault, zethAmount);
emit Events.Deposit(bridge, msg.sender, zethAmount);
}

  • depositzETH:

function depositEth(address bridge)
external
payable
nonReentrant
onlyValidBridge(bridge)
{
if (msg.value < Constants.MIN_DEPOSIT) revert Errors.UnderMinimumDeposit();
​
uint256 vault;
if (bridge == rethBridge || bridge == stethBridge) {
vault = Vault.CARBON;
} else {
vault = s.bridge[bridge].vault;
}
​
uint88 zethAmount = uint88(IBridge(bridge).depositEth{value: msg.value}()); // Assumes 1 ETH = 1 ZETH
vault.addZeth(zethAmount);
maybeUpdateYield(vault, zethAmount);
emit Events.DepositEth(bridge, msg.sender, zethAmount);
}

  1. In these functions, the line vault.addZeth is supposed to increase the user balance. Look at it's code:

function addZeth(uint256 vault, uint88 amount) internal {
AppStorage storage s = appStorage();
s.vaultUser[vault][msg.sender].ethEscrowed += amount;
s.vault[vault].zethTotal += amount;
}

  1. However, the line s.vaultUser[vault][msg.sender].ethEscrowed += amount; is increasing the ethEscrowed of msg.sender, which is the BridgeRouterFacet.sol, so the user will deposit a X amount, but the only address which will get ethEscrowed is BridgeRouterFacet.sol.

Impact

Probability: High.
Severity: High .
Impact: High .

Any user that deposit via deposit or depositEth in BridgeRouterFacet.sol lose 100% of funds, so it's a high severity vulnerability.

Tools Used

Manual Review

Recommendations

Add a parameter in addZeth that accepts an address. Then, vault.addZeth should input the user's address and addZeth should not rely anymore in msg.sender.

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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