Hubble contest - cccz's results

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/Oracle.sol#L33

Vulnerability details

Impact

On Oracle.sol, we are using latestRoundData, but there is no check if the return value indicates stale data.

(,answer,,,) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();

This could lead to stale prices according to the Chainlink documentation:

https://docs.chain.link/docs/historical-price-data/#historical-rounds https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/Oracle.sol#L33

Tools Used

None

Recommended Mitigation Steps

Consider adding missing checks for stale data.

For example:

      (uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
      require(answeredInRound >= roundID, "Stale price");
      require(timestamp != 0,"Round not complete");
      require(answer > 0,"Chainlink answer reporting 0");

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/VUSD.sol#L7

Vulnerability details

Impact

Same as https://github.com/code-423n4/2022-01-livepeer-findings/issues/193

The VUSD contract inherits from the ERC20PresetMinterPauser contract

Using the mint() function of ERC20PresetMinterPauser, an address with MINTER_ROLE can mint an arbitrary amount of tokens.

If the private key of the deployer or an address with the MINTER_ROLE is compromised, the attacker will be able to mint an unlimited amount of VUSD tokens.

We believe this is unnecessary and poses a serious centralization risk.

Proof of Concept

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/VUSD.sol#L7

Tools Used

None

Recommended Mitigation Steps

Consider removing the MINTER_ROLE, make the VUSD only mintable by the owner, and make the MarginAccount contract to be the owner and therefore the only minter.