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

The check inside `createVault()` for presence of already created vaults is insufficient

t0x1c

Summary & Vulnerability Details

Doc mentions:

  • There is only a single Vault at launch, but a new Vault can be made with createVault

The current carbon zETH token is mapped to Vault.CARBON having vault id as 1. This is through the mapping: mapping(address zeth => uint256 vault) zethVault. The check in createVault() ensures same carbon zETH token can not map to another vault.
A new vault should be created with a new zETH token.

File: contracts/facets/OwnerFacet.sol
​
135 function createVault(
136 @> address zeth,
137 uint256 vault,
138 MTypes.CreateVaultParams calldata params
139 ) external onlyDAO {
140 @> if (s.zethVault[zeth] != 0) revert Errors.VaultAlreadyCreated();
141 s.zethVault[zeth] = vault;
142 _setTithe(vault, params.zethTithePercent);
143 _setDittoMatchedRate(vault, params.dittoMatchedRate);
144 _setDittoShorterRate(vault, params.dittoShorterRate);
145 emit Events.CreateVault(zeth, vault);
146 }

Imagine the protocol owner or DAO wants to create a new vault with a new token. Let's call this nzETH (new zETH). So DAO would ideally call the function in the following way to create a new vault:

MTypes.CreateVaultParams memory newVaultParams;
createVault(address(nzETH), 5, newVaultParams);

This would create a new vault with vault id 5. No problems here.

Suppose now DAO wants to create another new vault for an another token pzETH (popular zETH). He wishes to use vault id 8 for this purpose. He calls the following, making a user input error:

MTypes.CreateVaultParams memory popularVaultParams;
createVault(address(pzETH), 5, popularVaultParams); // @audit-info : DAO passes a pre-existing vault id (5) by mistake!

Impact

The impact of this input error is high:

  • First, since the code only checks if (s.zethVault[address(pzETH)] != 0) revert Errors.VaultAlreadyCreated();, this won't revert and pzETH will be mapped to vault id in the next line via s.zethVault[address(pzETH)] = 5;.

    Vault 5 now has 2 tokens. Also, params for vault 5 like tithe, dittoMatchedRate & dittoShorterRate are now reset using popularVaultParams instead of the old newVaultParams.
    This could be problematic because now an external user can call depositZETH() with pzETH.

depositZETH(address(pzETH), 1000_000);

and this will pass all the checks there, burn some pzETH from external user's wallet, and increase user's ethEscrowed as if he deposited carbon zETH.

  • The second impact is: Since there is no unmapTokenFromVault() or similar function in the protocol, there is no way for DAO now to create another vault using pzETH. Calling createVault() will fail at if (s.zethVault[address(pzETH)] != 0) revert Errors.VaultAlreadyCreated(); since it has a value of 5 now.
    He also can not "unmerge" these 2 tokens within the vault.


Owner's input error --> High impact on the protocol with no way to correct it, hence raising as medium severity.



PoC

Paste the following code inside test/Owner.t.sol and run via forge test --mt test_InsufficientCheckInCreateVault -vv. The test would revert with Reason: VaultAlreadyCreated() -

function test_InsufficientCheckInCreateVault() public {
vm.startPrank(owner);
​
// Create a new vault with id 5 for nzETH (new zETH)
address nzETHaddress = makeAddr("nzETH");
MTypes.CreateVaultParams memory newVaultParams;
diamond.createVault(nzETHaddress, 5, newVaultParams);
​
// Create a new vault with id 5 for nzETH (new zETH)
address pzETHaddress = makeAddr("pzETH");
MTypes.CreateVaultParams memory popularVaultParams;
// input error - passing vault id as 5 again. Will NOT revert with `Errors.VaultAlreadyCreated`.
diamond.createVault(pzETHaddress, 5, popularVaultParams);
​
// DAO can't correct his mistake now
diamond.createVault(pzETHaddress, 8, popularVaultParams); // passing correct vault id
​
vm.stopPrank();
}

Tools Used

Manual inspection, forge test.

Recommendations

Maintain a mapping of already created vault ids and check that too.

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

Support

FAQs

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