code-423n4 / 2023-05-venus-findings

2 stars 1 forks source link

Incorrect decimal handling in `_startAuction`, resulting in wrong `auction.startBidBps` #518

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Shortfall/Shortfall.sol#L393

Vulnerability details

Shortfall._startAuction uses the oracle price of the underlying tokens to price the pool bad debt:

Shortfall.sol
389: for (uint256 i; i < marketsCount; ++i) {
390:             uint256 marketBadDebt = vTokens[i].badDebt();
391: 
392:             priceOracle.updatePrice(address(vTokens[i]));
393:             uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / 1e18;
394: 
395:             poolBadDebt = poolBadDebt + usdValue;

As per https://github.com/VenusProtocol/oracle/blob/953939c5dc2c556e8423d1b810f93c2e01931916/contracts/ResilientOracle.sol#L302 the price returned by priceOracle.getUnderlyingPrice has a number of decimals equal to 36 - vToken decimals. This means the usdValue line 393 has: 18 - vToken.decimals + underlying.decimals (marketBadDebt is an amount of underlying). This is incorrect, usdValue should be in underlying.decimals

Impact

The calculations line 405-408 are incorrect, resulting in an incorrect auction.startBidBps

Tools Used

Manual Analysis

Recommended Mitigation Steps

Refactor usdValue so that it has underlying.decimals

Assessed type

Math

0xean commented 1 year ago

Wardens logic is a bit hard to follow and does not include a coded POC to show why this is "incorrect". Even an example with values would go a long way in making this a higher quality submission. Will leave open for sponsor review, but ultimately probably close as insufficient quality

c4-sponsor commented 1 year ago

chechu marked the issue as sponsor disputed

chechu commented 1 year ago

usdValue should be in 18 decimal places not underlying.decimals

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Insufficient proof