Removing collateral can be exploited due to rounding issues in Buckets.collateralToLP()
Summary
When the Buckets.collateralToLP function is called, there can be rounding issues due to the fact that the rounding_ param is being ignored in certain cases. This can be exploited by users when removing collateral.
Vulnerability Detail
When the Buckets.collateralToLP function is called, there are 2 cases where the rounding_ param is ignored, which is used to round in favor of the protocol. But since the rounding_ param is ignored, rounding in favor of the user can occur.
Case 1: When there's no deposit nor collateral in bucket the result of the function Maths.wmul is returned:
The function Maths.wmul is rounding up or down to the nearest WAD = 10**18. The rounding can be in favor of or against the user, which is not good, because rounding should always be against the user and in favor of the protocol.
Case 2: When there's deposit or collateral in bucket but no LP to cover, the result of the function Maths.wmul is returned again similar to Case 1:
When removing collateral, the lpAmount_ should be rounded up in favor of the protocol, but can be rounded down in favor of the user due to the issues described above inside Buckets.collateralToLP():
The same applies for the use case where the max amount of collateral is removed and the requiredLP should be rounded up in favor of the protocol, but can be rounded down in favor of the user due to the issues described above inside Buckets.collateralToLP():
Note that the user sets the amount_ of collateral (LenderActions.sol line 467), which is then passed into Buckets.collateralToLP() (LenderActions.sol line 487) for the collateral_ param (Buckets.sol line 115) which is used for the Maths.wmul calculations (Buckets.sol line 120, 123):
That means that the user can control the rounding behaviour of Maths.wmul by selecting an appropriate amount of collateral to remove, so that the result of Buckets.collateralToLP is rounded down in favor of the user.
Impact
The protocol is potentially rounding in favor of the user when removing collateral or when removing the max amount of collateral, which can be exploited.
lemonmon
high
Removing collateral can be exploited due to rounding issues in
Buckets.collateralToLP()Summary
When the
Buckets.collateralToLPfunction is called, there can be rounding issues due to the fact that therounding_param is being ignored in certain cases. This can be exploited by users when removing collateral.Vulnerability Detail
When the
Buckets.collateralToLPfunction is called, there are 2 cases where therounding_param is ignored, which is used to round in favor of the protocol. But since therounding_param is ignored, rounding in favor of the user can occur.Case 1: When there's no deposit nor collateral in bucket the result of the function
Maths.wmulis returned:https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L120
The function
Maths.wmulis rounding up or down to the nearestWAD = 10**18. The rounding can be in favor of or against the user, which is not good, because rounding should always be against the user and in favor of the protocol.Case 2: When there's deposit or collateral in bucket but no LP to cover, the result of the function
Maths.wmulis returned again similar to Case 1:https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L123
When removing collateral, the
lpAmount_should be rounded up in favor of the protocol, but can be rounded down in favor of the user due to the issues described above insideBuckets.collateralToLP():https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L483
The same applies for the use case where the max amount of collateral is removed and the
requiredLPshould be rounded up in favor of the protocol, but can be rounded down in favor of the user due to the issues described above insideBuckets.collateralToLP():https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L674
Note that the user sets the
amount_of collateral (LenderActions.sol line 467), which is then passed intoBuckets.collateralToLP()(LenderActions.sol line 487) for thecollateral_param (Buckets.sol line 115) which is used for theMaths.wmulcalculations (Buckets.sol line 120, 123):https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L467
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L487
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L115
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L120
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L123
That means that the user can control the rounding behaviour of
Maths.wmulby selecting an appropriate amount of collateral to remove, so that the result ofBuckets.collateralToLPis rounded down in favor of the user.Impact
The protocol is potentially rounding in favor of the user when removing collateral or when removing the max amount of collateral, which can be exploited.
Code Snippet
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L115
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L120
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/internal/Buckets.sol#L123
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L467
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L483
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L487
https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/libraries/external/LenderActions.sol#L674
Tool used
Manual Review
Recommendation
In the function
Buckets.collateralToLPdon't ignore therounding_param.