code-423n4 / 2021-04-vader-findings

1 stars 0 forks source link

Replacing an anchor does not reset `Pool.isAnchor` #212

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

@cmichelio

Vulnerability details

Vulnerability Details

The Router.replaceAnchor function can be used to replace an anchor, however Pool._isAmchor is not reset.

Impact

Price updates are still attempted on each swap through _handleAnchorPriceUpdate which costs lots of unnecessary gas.

Recommended Mitigation Steps

It's not clear to the auditor how the whole _.isAnchor system is supposed to work as any token can be made an anchor through a Pools.addLiquidity call.

strictly-scarce commented 3 years ago

It's subtle, but there are two types of Anchors

1) Those with VADER as base pair 2) A subset of (1) that are enabled for pricing of the Anchor

If not is (2) then this will skip, since it won't be in the array of anchors listed for pricing:

// Anyone to update prices
    function updateAnchorPrice(address token) public {
        for (uint256 i = 0; i < arrayAnchors.length; i++) {
            if (arrayAnchors[i] == token) {
                arrayPrices[i] = iUTILS(UTILS()).calcValueInBase(arrayAnchors[i], one);
            }
        }
    }
dmvt commented 3 years ago

This appears to be intended functionality per sponsor, but the functionality is confusing for anyone other than the original devs. This can and should be addressed with commenting and documentation.