code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L544-L551

Vulnerability details

Impact

The function addDust is used in closeDebt to // Add dust to be sure debt reformed >= debt outstanding as stated in the NatSpec, but in the cases of tokens with less decimals, the amount calculated will be way bigger than expected which could make the whole function revert.

Proof of Concept

addDust is used to calculate the debt variable by adding the repayAmount with the value returned from addDust, as can be seen here https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L265 The repayAmount would be a value which will have the decimal places of variableDebtTokenAddress, which is the same as the variable debt tokens from AAVE, since it is an AAVE fork. The variable debt tokens form AAVE have the same decimal places as the underlying assets, so because same thing will be here. Let's assume that the variableDebtTokenAddress of the debt intended to be payed is from USDC.e, a token which the protocol intends to use as per Scoping Details, and has 6 decimals places. TherepayAmountin this case would be a 1e6 type number andaddDust` would be calculated with one of the tokens being USDC so the calculations will look like this :

let's assume that the other token used have 18 decimals so the scale0 here https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L546 would be a 1e2 type number, since getAssetPrice returns a 1e8 number.
scale1 would be the USDC one for this case https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L547 so the calculation would be a 1e14, since getAssetPrice returns again a 1e8 number, so 1e14 * 1e8 / 1e8 = ~1e14
in the last if statement the bigger value will always be returned https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L549-L550 so in this case 1e14
because of that the debt would be repayAmount which is a 1e6 number + a 1e14 number from addDust, which will make the debt be way bigger than intended, reverting most of the time on checkExpectedBalances, or in very bad cases making the user pay more than intended

Tools Used

Manual review

Recommended Mitigation Steps

Change the logic of addDust function since it add more than just dust in the case of where one of the tokens has less decimals.

Assessed type

Math

c4-sponsor commented 1 year ago

Keref marked the issue as sponsor disputed

Keref commented 1 year ago

POC shows repaying a 6 decimals aUSDC debt with a method for TokenisableRange, which always have 18 decimals.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-08-goodentry-findings

`addDust` in `OptionsPositionManager.sol` can add way more tokens than expected in cases where the asset used has less decimals #548