Security Audit Report

Dynamo Yield Vault Audit Report

Reviewed by: 0x52 (@IAm0x52)

Prepared For: Dynamo Finance

Review Date(s): 7/10/23 - 7/12/23

Fix Review Date(s): 7/25/23

0x52 Background

As a professional smart contract auditor, I have conducted over 100 security reviews for public and private clients. With 30+ first-place finishes in public contests on platforms like Code4rena and Sherlock, I have been recognized as a top-performing security expert. By prioritizing rigorous analysis and providing actionable recommendations, I have contributed to securing over $1 billion in TVL across 100+ protocols. Throughout my career I have collaborated with many organizations including the prestigious Blackthorn as a founding security researcher and as a Lead Security researcher at SpearbitDAO.

Protocol Summary

Dynamo Finance is a yield vault protocol that allows for adaptive fund allocation across different lending protocols.

Scope

Repo: Dynamo

Review Hash: c331ffefad

Fix Review Hash:

In-Scope Contracts

• contracts/aaveAdapter.vy
• contracts/compoundadapter.vy
• contracts/Dynamo4626.vy
• contracts/fraxlendadapter.vy
• contracts/FundsAllocator.vy
• contracts/Governance.vy

Deployment Chain(s)

• Ethereum Mainnet

Summary of Findings

High Risk Findings

Access control modifiers on _claim_fees will permanently lock proposer

Details

Dynamo4626#_set_strategy attempts to distribute fees to proposer when proposer changes. The problem is that _claim_fees requires that msg.sender == proposer. Since _set_strategy can only be called by governance this subcall will always revert. The result is that the first proposer will have a monopoly on all proposals since any strategy that wasn't submitted by them would fail when attempting to activate it.

Lines of Code

Dynamo4626.vy#L519-L521

    elif _yield == FeeType.PROPOSER:
        assert msg.sender == self.current_proposer, "Only curent proposer may claim strategy fees."
        self.total_strategy_fees_claimed += claim_amount        

Recommendation

Revise access control on _set_strategy. I would suggest allowing anyone to claim tokens but sending to the correct target instead of msg.sender

Remediation

Fixed here by allowing governance to claim on behalf of proposer

Math error in Dynamo4626#_claimable_fees_available will lead to fees or strategy lockup

Details

In the assert statement, total_fees_ever is compared against both fees types of fees claimed. The issue with this is that this is a relative value depending on which type of fee is being claimed. The assert statement on the other hand always compares as if it is FeeType.BOTH. This will lead to this function unexpectedly reverting when trying to claim proposer fees. This leads to stuck fees but also proposer locked as described in H-01.

Lines of Code

Dynamo4626.vy#L428-L438

    fee_percentage: uint256 = YIELD_FEE_PERCENTAGE
    if _yield == FeeType.PROPOSER:
        fee_percentage = PROPOSER_FEE_PERCENTAGE
    elif _yield == FeeType.BOTH:
        fee_percentage += PROPOSER_FEE_PERCENTAGE
    elif _yield != FeeType.YIELD:
        assert False, "Invalid FeeType!" 

    total_fees_ever : uint256 = (convert(total_returns,uint256) * fee_percentage) / 100

    assert self.total_strategy_fees_claimed + self.total_yield_fees_claimed <= total_fees_ever, "Total fee calc error!"

Recommendation

Check should be made against the appropriate values (i.e. proposer should be check against only self.total_strategy_fees_claimed).

Remediation

Fixed here and here by splitting fee calculations base on fee type being claimed

Malicious user can disable compound integration via share manipulation

Details

It's a common assumption that Compound V2 share ratio can only ever increase but with careful manipulation it can actually be lowered. The full explanation is a bit long but you can find it here in one of my public reports.

This quirk of Compound V2 can be used to trigger the check in FundsAllocator to block the Compound V2 adapter. This is useful if the user wants to push their own proposal allowing them to sabotage other users and cause loss of yield to the vault.

Lines of Code

FundsAllocator.vy#L67-L71

          if pool.current < pool.last_value:
              # We've lost value in this adapter! Don't give it more money!
              blocked_adapters[blocked_pos] = pool.adapter
              blocked_pos += 1
              pool.delta = 0 # This will result in no tx being generated.

Recommendation

Instead of using an absolute check, instead only block the adapter if there is reasonable loss.

Remediation

Fixed here by allowing larger nominal (but still very small) loss before disabling it

Dangerous approval/rejection criteria when number of guards is odd

Details

The assert statement requires that the number of endorsements equals or exceeds the number of guards / 2. This becomes an issue with odd numbers due to truncation. If you were to have 3 guards then even a single approval would allow instant approval (3/2 = 1). In this scenario even a single malicious or compromised guard could drain the entire vault via a malicious proposal.

Lines of Code

Governance.vy#L310

    assert (len(pending_strat.VotesEndorse) >= len(self.LGov)/2) or \

Recommendation

Make the requirement that it must equal or exceed length / 2 + length % 2

Remediation

Fixed here by changing len(self.LGov)/2 to len(self.LGov)/2 + 1 so that a majority vote is required to pass

A single malfunctioning/malicious adapter can permanently DOS entire vault

Details

When rebalancing the vault, FundsAllocator attempts to withdraw/deposit from each adapter. In the event that the underlying protocol (such as AAVE) disallows deposits or withdrawals (or is hacked), the entire vault would be DOS'd since rebalancing is called on every withdraw, deposit or strategy change.

Lines of Code

FundsAllocator.vy#L47-L57

    for pos in range(MAX_POOLS):
        pool : BalancePool = _pool_balances[pos]
        if pool.adapter == empty(address): break

        # If the pool has been removed from the strategy then we must empty it!
        if pool.ratio == 0:
            pool.target = 0
            pool.delta = convert(pool.current, int256) * -1 # Withdraw it all!
        else:
            pool.target = (total_pool_target_assets * pool.ratio) / _total_ratios      
            pool.delta = convert(pool.target, int256) - convert(pool.current, int256)            

Recommendation

Add an emergency function to force remove adapters and make it accessible via Governance.vy

Remediation

Fixed here by simply bypassing failed adapter calls

Medium Risk Findings

Governance#replaceGovernance is unable to actually change vault governance

Details

The code to actually replace the governance contract has been commented out resulting in it being impossible to ever change the governance contract.

Lines of Code

Governance.vy#L472-L477

    for guard_addr in self.LGov:
        if self.VotesGCByVault[vault][guard_addr] == NewGovernance:
            VoteCount += 1

    # if len(self.LGov) == VoteCount:
    #     Vault(self.Vault).replaceGovernanceContract(NewGovernance)

Recommendation

Restore code by removing comment

Remediation

Fixed here by removing comment

Assert statement in Dynamo4626#_claimable_fees_available can cause vault softlock in the event of partial fund loss

Details

In the event of partial fund loss there may be legit cases where this assert statement is triggered. If the vault suffers a partial loss but still maintains a positive return (i.e. it has made 100e18 but suffers a loss of 50e18) then this statement will improperly revert. Given this function is called with every deposit and withdraw the vault would be completely DOS'd until yield (or donation) recovered the difference.

Lines of Code

Dynamo4626.vy#L438

    assert self.total_strategy_fees_claimed + self.total_yield_fees_claimed <= total_fees_ever, "Total fee calc error!"

Recommendation

Instead of reverting, simply return 0.

Remediation

Fixed here by returning 0 instead of reverting

Low Risk Findings

aaveAdapter.vy has no method to claim LP incentives

Details

Considering this low since the primary scope for this audit is mainnet Ethereum.

On alt L1's and L2's AAVE V3 frequently has LP incentives (such as OP tokens on Optimism). The current adapter has no methodology to claim these tokens. Any tokens accumulated to the vault would be impossible to claim leading to loss of yield for all LP's in the vault.

Lines of Code

aaveAdapter.vy

Recommendation

Before deploying anything other than mainnet make sure to include a way to claim and distribute/swap rewards to the vault.

Remediation

Acknowledged that this is a limitation of their current implementation

Ownership of governance.vy can't be changed after initialization

Details

The ability to change the governance contract's owner is not implemented, which could be problematic if the original owner's key is compromised or lost.

Lines of Code

Governance.vy

Recommendation

Implement functionality to change owner of the governance contract.

Remediation

Fixed in later version.