Gravita-Protocol / Gravita-SmartContracts

GNU General Public License v3.0
48 stars 31 forks source link

Wrong else if() statement could break the protocol rules . #289

Closed pavankv241 closed 1 year ago

pavankv241 commented 1 year ago

Affected Smart contracts VesselManagerOperations.sol

Impact

Severity High because it breaks the protocol rules and loss to liquidator .

code snippet:- https://github.com/Gravita-Protocol/Gravita-SmartContracts/blob/main/contracts/VesselManagerOperations.sol#L706-L728

Description :- Document states that .

GRAI in the Stability Pool equal to the Vessel's debt is offset with the Vessel's debt. The Vessel's collateral (minus compensation) is shared between depositors.

The total Stability Pool GRAI is offset with an equal amount of debt from the Vessel. A fraction of the Vessel's collateral (equal to the ratio of its offset debt to its entire debt) is shared between depositors. The remaining debt and collateral (minus compensation) is redistributed to active Vessels.

But in line 706 code looks like even [SP GRAI > Vessel debt] it will calculate fraction of vessel collateral will be shared depsositors through _getOffsetAndRedistributionVals() and comments states like below :-

If the vessel's debt is larger than the deposited debt token in the Stability Pool:

And there is no check in _getOffsetAndRedistributionVals() to whether vessel's debt is larger than the deposited debt token in the Stability Pool . It simply calculates even [SP GRAI > Vessel debt] .

Recommedation :- Change like this :-


else if ((_ICR > adminContract._100pct()) && (_ICR < adminContractCached.getMcr(_asset))) {
            vesselManagerCached.movePendingVesselRewardsToActivePool(_asset, vars.pendingDebtReward, vars.pendingCollReward);
            vesselManagerCached.removeStake(_asset, _borrower);

if( SP GRAI< Vessel debt ){ ///add  if statement .
            (
                singleLiquidation.debtToOffset,
                singleLiquidation.collToSendToSP,
                singleLiquidation.debtToRedistribute,
                singleLiquidation.collToRedistribute
            ) = _getOffsetAndRedistributionVals(
                singleLiquidation.entireVesselDebt,
                vars.collToLiquidate,
                _debtTokenInStabPool
            );

} ```
0xfornax commented 1 year ago

We appreciate the submission. The function works as intended: the liquidator is always guaranteed the debtTokenGasCompensation (which was set aside on Vessel creation) and a percentage of the vessel's collateral before distributing the debt o the Stability Pool (see https://github.com/Gravita-Protocol/Gravita-SmartContracts/blob/main/contracts/VesselManagerOperations.sol#L204). Similar issue to #283