DYAD - n0kto's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L64-L65 https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L93-L98

Vulnerability details

Description

During the deployment script, incorrect vaults are added to the Kerosene Manager and Vault Licenser:

@>        keroseneManager.add(address(ethVault));
@>        keroseneManager.add(address(wstEth));

        ...

@>        vaultLicenser.add(address(unboundedKeroseneVault));

@>        // vaultLicenser.add(address(boundedKeroseneVault));

This allows the same vault to be added twice via VaultManagerV2::add and VaultManagerV2::addKerosene. Any token will be counted twice in collatRatio due to getTotalUsdValue.

Furthermore, boundedKeroseneVault and unboundedKeroseneVault are not added to the correct manager.

Risk

Likelihood: High

• Any attacker can add the same vault as a normal vault and a kerosene vault.

Impact: High

• Any users under the collateral ratio won't be liquidated.

Recommended Mitigation

Only add necessary vaults to the correct licenser/manager. Implement a check to ensure each vault is only counted once in the collateral calculation.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L94-L104

Vulnerability details

Description

The remove function expects the user to not have any assets in the vault. However, someone can frontrun it and deposit 1 wei to prevent the user from removing a vault. The ability to remove a vault is crucial when adding new ones, as there is a maximum limit of 5.

    function remove(uint id, address vault) external isDNftOwner(id) {
@>        if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();
        if (!vaults[id].remove(vault)) revert VaultNotAdded();
        emit Removed(id, vault);
    }

Risk

Likelihood: Low

• An attacker would need to target a specific person and pay for gas.

Impact: High

• A user might be unable to remove a vault.

Recommended Mitigation

A user can add a previous vault and retrieve all their assets, so this condition is not necessary. Allow users to remove any vaults they want.

Assessed type

MEV