Assessment reports>Resonate>Critical findings>Incorrect asset tracking in modifyExistingOrder
Category: Business Logic

Incorrect asset tracking in modifyExistingOrder

Critical Severity
Critical Impact
High Likelihood

Description

For cross-asset pools, calling submit submitProducer(...) always sets shouldFarm = false and order.depositedShares > 0 using the oracle price. Orders are then enqueued with the pool asset:

if(shouldFarm) {
    IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
    order.depositedShares = IERC4626(vaultAdapter).deposit(amount, getAddressForPool(poolId)) / order.packetsRemaining; 
} else {
    IERC20(asset).safeTransferFrom(msg.sender, getAddressForPool(poolId), amount);
}

However, modifyExistingOrder(...) has missing checks and assumes the orders were deposited in the vault asset:

if (order.depositedShares > 0) {
    getWalletForPool(poolId).withdrawFromVault(amountTokens, msg.sender, vaultAdapters[pool.vault]);
} else { 
    getWalletForPool(poolId).withdraw(amountTokens, pool.asset, msg.sender);
}

Impact

An attacker could spam submitProducer(...) and modifyExistingOrder(...) to convert pool assets to vault assets at a rate of 1:1. This could be financially lucrative as there are no ways to shut down the protocol or pull other users funds. It would also disrupt of the balance of the cross-asset pair and hence the potential operation of the pool.

Recommendations

Include logic to enure the vault and pool assets are correctly tracked in cross asset pools.

Remediation

Revest has implemented the following solution in committ 00000000000:

if (order.depositedShares > 0 && IERC4626(vaultAdapters[pool.vault]).asset() == pool.asset)

We find their remediation adequately addresses the concerns of this finding.

Zellic © 2024Back to top ↑