Wrong shareChange() function (vToken.sol)

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/vToken.sol#L160

Vulnerability details

Impact

Users can get the wrong amount of vToken
=> Make users lose their fund

Proof of Concept

Base on the code in function shareChange() in vToken.sol
Assume that if oldShare = totalSupply > 0,

• newShares
  = (_amountInAsset * (_totalSupply - oldShares)) / (_assetBalance - availableAssets);
  = (_amountInAsset * (_totalSupply - _totalSupply)) / (_assetBalance - availableAssets);
  = 0

It make no sense, because if amountInAsset >> availableAssets, newShares should be bigger than oldShares, but in this case newShares = 0 < oldShares

Tools Used

manual review

Recommended Mitigation Steps

Modify the line from if (_totalSupply > 0) to if (_totalSupply - oldShares > 0)

[jn-lp]

Such a case is considered impossible due to the fact that it can only work with a 0xdead address

[moose-code]

Agree its not an issue as on initialization tokens are sent to the burn address making this unlikely.

However the orderer role could possibly burn the tokens held by the burn address causing this issue to happen

[JasoonS]

Agree with mitigation step:

Modify the line from if (_totalSupply > 0) to if (_totalSupply - oldShares > 0)

If it were impossible for tokens to be burned from the 0xdead address then this wouldn't be a concern.

So although extremely unlikely, this is valid.