code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

Sound adjusting of a position could fail #942

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L132

Vulnerability details

Impact

Increasing collateral and decreasing it's liquidation price, such that with new liquidation price and old collateral position is becoming unsound, leads to a revert. Such situation can happen often, imagine position that's fully minted and has market price of collateral close to liquidation, in this case, to keep position from liquidation, owner will add collateral and decrease liquidation price. Since it's fully minted, any decrease in liquidation price will lead to revert.

Proof of Concept

calling adjust with newPrice < price and newCollateral > collateralBalance() will lead to these consecutive calls https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L134 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L163 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L283 and could revert, since collateralReserve equals to CollateralBalance(), which wasn't updated yet https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Position.sol#L138

Recommended Mitigation Steps

Place if (newPrice != price) clause after if (newMinted < minted)

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #454

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory