The `bridgePointer` is not properly specified in `BridgeRouterFacet::_getVault`.

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibBridgeRouter.sol#L30 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BridgeRouterFacet.sol#L157

Vulnerability details

Impact

When depositing via BridgeRouterFacet::deposit, the user needs to specify the bridge and the amount. The issue arises when the user specifies the rethBridge, but in _getVault(), the bridgePointer variable is left uninitialized, resulting in bridgePointer=0:

File: BridgeRouterFacet.sol
157:         if (bridge == rethBridge) {
158:             vault = VAULT.ONE;
159:         }

As a result, bridgePointer is set to zero, causing the amount to be credited to bridgeCreditSteth instead of bridgeCreditReth in LibBridgeRouter#L30.

File: LibBridgeRouter.sol
30:                 VaultUser.bridgeCreditSteth += amount;

This discrepancy in accounting may lead to errors in tracking the deposited amount.

Tools used

Manual review

Proof of Concept

Recommended Mitigation Steps

To address this issue, ensure that the bridgePointer for the rethBridge is correctly specified in _getVault:

    function _getVault(address bridge) private view returns (uint256 vault, uint256 bridgePointer) {
        if (bridge == rethBridge) {
            vault = VAULT.ONE;
++          bridgePointer = VAULT.BRIDGE_RETH;
        } else if (bridge == stethBridge) {
            vault = VAULT.ONE;
            bridgePointer = VAULT.BRIDGE_STETH;
        } else {
            vault = s.bridge[bridge].vault;
            if (vault == 0) revert Errors.InvalidBridge();
        }
    }

By adding this modification, the system will correctly credit the deposited amount to bridgeCreditReth, ensuring accurate accounting.

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[raymondfam]

Seems valid. Will let sponsor look into it.

[c4-sponsor]

ditto-eth (sponsor) disputed

[ditto-eth]

the bridge pointer doesn't need to be set in this context because the pointer for bridgeReth is zero. see Constants.sol

library VAULT {
    // ONE is the default vault
    uint256 internal constant ONE = 1;
    // Bridges for Vault ONE
    uint256 internal constant BRIDGE_RETH = 0;
}

[ditto-eth]

additionally the initial premise of the issue is incorrect.

if (bridgePointer == VAULT.BRIDGE_RETH) {
    VaultUser.bridgeCreditReth += amount;
} else {
    VaultUser.bridgeCreditSteth += amount;
}

if the user specifies bridgeReth it will go the top if statement and not the else

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid