Vader Protocol contest - Jujic's results

Handle

Jujic

Vulnerability details

There is no upper limit on vaderPairs[], it increments each time when a new pair is added. Eventually, as the count of pair increases, gas cost of smart contract calls will raise and that there is no implemented function to reduce the array size.

Impact

For every call functions which computed the Vaden price of a given par is listed in vaderPairs array, the gas consumption can be more expensive each time that a new collateral address is appended to the array, until reaching an "Out of Gas" error or a "Block Gas Limit" in the worst scenario. The same situation with usdvPairs array.

As a result, this situation can lead to the blocking of the pricing mechanism and loss reimbursement in the protocol.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/28d3405447f0c3353964ca755a42562840d151c5/contracts/lbt/LiquidityBasedTWAP.sol#L301

https://github.com/code-423n4/2021-12-vader/blob/28d3405447f0c3353964ca755a42562840d151c5/contracts/lbt/LiquidityBasedTWAP.sol#L488

Tools Used

Remix

Recommended Mitigation Steps

Add additional check for max array length or add remove function

Handle

Jujic

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Proof of Concept

function _onlyUSDV() private view {
        require(
            address(usdv) == msg.sender,
            "Vader::_onlyUSDV: Insufficient Privileges"
        );
 }

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/Vader.sol#L225

Tools Used

Remix

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

Handle

Jujic

Vulnerability details

Impact

State variable vadger is read two times respectively in mint() function. Caching it in local variable at the beginning of the function and using local variables can save gas.

Proof of Concept

vader.transferFrom(msg.sender, address(this), vAmount);
vader.burn(vAmount);

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/USDV.sol#L73-L74

Tools Used

Remix

Recommended Mitigation Steps

Cache vader state variable in local variable at the beginning of the function and use those local variable instead.

Handle

Jujic

Vulnerability details

Impact

Using the unchecked keyword to avoid redundant arithmetic underflow/overflow checks to save gas when an underflow/overflow cannot happen.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/USDV.sol#L79

cycleTimestamp = block.timestamp + 24 hours;

Tools Used

Remix

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so.

unchecked {
         cycleTimestamp = block.timestamp + 24 hours;
 }

Handle

Jujic

Vulnerability details

Impact

modifier onlyUSDV() calls internal function _onlyUSDV(). This function is not called anywhere else. So I do not see a reason why all the logic can't be moved to the modifier to save some gas by reducing the extra call.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/Vader.sol#L222-L237

Tools Used

Remix

Recommended Mitigation Steps

Remove function _onlyUSDV(), place its logic directly in the onlyUSDV() modifier.

Handle

Jujic

Vulnerability details

Impact

The method .latestRoundData() on an oracle returns the latest updated price from the oracle, but this is not the current price of an asset. To get an accurate current price you need to query it by calling the oracle and waiting for a callback to fulfill the request.

Inaccurate price data could quickly lead to a large loss of funds. Suppose the price of an asset changes downward 5% but your oracle is not updated.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/lbt/LiquidityBasedTWAP.sol#L86

Tools Used

Remix

https://github.com/code-423n4/2021-08-notional-findings/issues/18

Recommended Mitigation Steps

Recommend not fetching the latest price (having to call the oracle to update the price instead), and then waiting for the callback.