search

code-423n4 / 2022-02-hubble-findings

2 stars 2 forks source link

`Oracle.getUnderlyingPrice` could have wrong decimals #44

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/8c157f519bc32e552f8cc832ecc75dc381faa91e/contracts/Oracle.sol#L34

Vulnerability details

Impact

The Oracle.getUnderlyingPrice function divides the chainlink price by 100. It probably assumes that the answer for the underlying is in 8 decimals but then wants to reduce it for 6 decimals to match USDC.

However, arbitrary underlying tokens are used and the chainlink oracles can have different decimals.

Recommended Mitigation Steps

While most USD price feeds use 8 decimals, it's better to take the on-chain reported decimals into account by doing AggregatorV3Interface(chainLinkAggregatorMap[underlying]).decimals(), see Chainlink docs. The price should then be scaled down to 6 decimals.

atvanguard commented 2 years ago

All chainlink USD pairings are expected to have 8 decimals hence disagreeing with severity; but yes agree that asserting this check when adding a new asset is a good idea.

moose-code commented 2 years ago

Downgrading to medium. Dividing by magic numbers (100) should clearly comment assumptions