Vader Protocol contest - gzeon's results

Handle

gzeon

Vulnerability details

Impact

addVaderPair does not check if the pair is already added, duplicates can be added which will lead to double-counting of liquidity.

Notably the pair will be added to vaderPairs which will be iterated over in e.g. syncVaderPrice https://github.com/code-423n4/2021-12-vader/blob/9fb7f206eaff1863aeeb8f997e0f21ea74e78b49/contracts/lbt/LiquidityBasedTWAP.sol#L301

        vaderPairs.push(pair);

https://github.com/code-423n4/2021-12-vader/blob/9fb7f206eaff1863aeeb8f997e0f21ea74e78b49/contracts/lbt/LiquidityBasedTWAP.sol#L122

        uint256 totalPairs = vaderPairs.length;
        pastLiquidityWeights = new uint256[](totalPairs);
        pastTotalLiquidityWeight = totalLiquidityWeight[uint256(Paths.VADER)];

        for (uint256 i; i < totalPairs; ++i) {

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/9fb7f206eaff1863aeeb8f997e0f21ea74e78b49/contracts/lbt/LiquidityBasedTWAP.sol#L238

Recommended Mitigation Steps

Check if the pair already exists in vaderPairs

Handle

gzeon

Vulnerability details

Impact

There are no way to remove/change existing pair in LiquidityBasedTWAP. This will become an issue if any of the oracle is down for extended period since most interaction with the contract will invoke getChainlinkPrice on all foreign asset, a broken oracle will revert all tx and make the protocol unusable.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/9fb7f206eaff1863aeeb8f997e0f21ea74e78b49/contracts/lbt/LiquidityBasedTWAP.sol#L88-93

        require(
            answeredInRound >= roundID,
            "LBTWAP::getChainlinkPrice: Stale Chainlink Price"
        );

        require(price > 0, "LBTWAP::getChainlinkPrice: Chainlink Malfunction");

Recommended Mitigation Steps

Add a method for removing USDVPair

Handle

gzeon

Vulnerability details

Impact

cycleMints is not checked against dailyLimit if it is the first mint in 24 hours. It allow minting more than dailyLimit which open up opportunity for an attacker to manipulate price, mint a lot of USDV and sell to liquidity pools.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/9fb7f206eaff1863aeeb8f997e0f21ea74e78b49/contracts/tokens/USDV.sol#L78 There are no check if cycleTimestamp <= block.timestamp

        if (cycleTimestamp <= block.timestamp) {
            cycleTimestamp = block.timestamp + 24 hours;
            cycleMints = uAmount;
        } else {
            cycleMints += uAmount;
            require(
                cycleMints <= dailyLimit,
                "USDV::mint: 24 Hour Limit Reached"
            );
        }

Recommended Mitigation Steps

Move the require statement out of the ifelse block