No check if bridge already exists
In the current `createBridge` function of the OwnerFacet.sol contract, a critical check to verify if the bridge already exists is missing. This omission can potentially result in double accounting in the yield generation process.\\nIn the rest of the OwnerFacet.sol contract functionality, there are checks in place to prevent the recreation of Vaults or Markets. However, this essential check is absent in the `createBridge()` function. The absence of this check can lead to the unintended creation of duplicate bridges, resulting in double accounting of yield if multiple vaults utilize the same bridge more than once. You can find the missing check in the code here: Link to code.\\nThe potential for double accounting of yield is evident in the following code block:\\n```\\nfunction getZethTotal(uint256 vault) internal view returns (uint256 zethTotal) {\\n    AppStorage storage s = appStorage();\\n    address[] storage bridges = s.vaultBridges[vault];\\n    uint256 bridgeCount = bridges.length;\\n\\n    for (uint256 i; i < bridgeCount;) {\\n        zethTotal += IBridge(bridges[i]).getZethValue(); \\n        unchecked {\\n            ++i;\\n        }\\n    }\\n}\\n```\\n\\nTo demonstrate this behavior, a simple Proof of Concept (PoC) was created. (The test was placed in the Yield.t.sol file.)\\n```\\nfunction test_double_bridge_push() public {\\n    vm.prank(owner);\\n    diamond.createBridge(_bridgeReth, Vault.CARBON, 0, 0);\\n    diamond.getUndistributedYield(Vault.CARBON); \\n    assert(diamond.getUndistributedYield(Vault.CARBON) > 0); // As no yield was generated, this should not be true, but in current situation, it is a proof of double accounting.\\n}\\n```\\n
No check if bridge already exists\\nTo address this vulnerability, it is recommended to add the following mitigation to the createBridge function:\\n```\\n// rest of code\\n// Add the line below\\n for (uint i = 0; i < s.vaultBridges[vault].length; i// Add the line below\\n// Add the line below\\n) {\\n// Add the line below\\n     if (s.vaultBridges[vault][i] == bridge) {\\n// Add the line below\\n         revert Errors.BridgeAlreadyExist();\\n// Add the line below\\n     }\\n// Add the line below\\n }\\n```\\n\\nThis change will prevent the inadvertent creation of duplicate bridges and mitigate the risk of double accounting of yield.
In specific circumstances, if a DAO proposal is confirmed, it could inadvertently trigger the creation of a bridge with the same address for a vault that already uses it. This scenario can lead to double accounting of yield and, as a consequence, potentially expose the protocol to vulnerabilities such as Denial of Service and yield theft.\\nHowever, it's important to note that the likelihood of this issue occurring is relatively low, and the function is governed by the DAO. After discussing this with the sponsor, we have classified this finding as low severity.
```\\nfunction getZethTotal(uint256 vault) internal view returns (uint256 zethTotal) {\\n    AppStorage storage s = appStorage();\\n    address[] storage bridges = s.vaultBridges[vault];\\n    uint256 bridgeCount = bridges.length;\\n\\n    for (uint256 i; i < bridgeCount;) {\\n        zethTotal += IBridge(bridges[i]).getZethValue(); \\n        unchecked {\\n            ++i;\\n        }\\n    }\\n}\\n```\\n