Unsafe casting of reserveAmount from uint256 to int256
Summary
Unsafe casting of reserveAmount from uint256 to int256, leading to an incorrect value. As a result, it could lead to DOS or a loss of assets, as shown in the report below.
Root Cause
Unsafe casting of reserveAmount from uint256 to int256, leading to an incorrect value
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
At Line 118 below, the code performs an unsafe downcasting from uint256 to int256. If the reserveAmount exceeds the maximum value of an int256 (2^255 - 1), the conversion will not revert automatically. Instead, it will silently result in an incorrect wrapped value.
Note: It is possible for the reserveAmount to exceed 2^255 - 1 within the protocol as the depositToReserveVault function does not restrict users from depositing an amount larger than 2^255 - 1 and accountLayout.reserveVault[partyB] is uint256.
The following demonstrates a scenario where a reserveAmount that is larger than int256 (2^255 - 1) resulting in the condition (partyBAvailableBalance + int256(reserveAmount)) at Line 118 above to be evaluated to be incorrect. When reserveAmount is a positive value of 2**256 - 100, it will become a negative value (-100) when downcasted from uint256 to int256, as shown below.
➜ ~ chisel
Welcome to Chisel! Type `!help` to show available commands.
➜ uint256 reserveAmount = 2**256 - 100;
➜ int256 reserveAmountDowncast = int256(reserveAmount)
➜ reserveAmountDowncast
Type: int256
├ Hex: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff9c
├ Hex (full word): 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff9c
└ Decimal: -100
➜ int256 partyBAvailableBalance = -150
➜ int256 result = partyBAvailableBalance + reserveAmountDowncast
➜ result
Type: int256
├ Hex: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff06
├ Hex (full word): 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff06
└ Decimal: -250
If there is no downcasting error, the condition at Line 118 should have been evaluated as True since:
However, as shown in the chisel output above, the final result is a negative number (result = partyBAvailableBalance + reserveAmountDowncast = -250), which leads to the condition being incorrectly evaluated as false.
This PartyB, with a large amount of reserve (2**256 - 100), should allow any force close action against them to be executed smoothly (without reverting or being liquidated) by going into the second branch of the if-else branch (Code in Lines 118-126).
However, due to the downcasting error, the logic in the final-else branch is executed instead (Lines 128-137 above), which contains the code for liquidating the PartyB (Line 127 above). Eventually, it is likely that the liquidation against PartyB will revert as the availableBalance within the LibLiquidation.availableBalance function will likely be larger than 0. This also means that PartyA cannot force close this position due to this revert.
On the other hand, it is possible that the liquidation process will go through without a revert (If the availableBalance within liquidatePartyB function increases, but still remains as a negative value). In this case, the consequences are as follows:
PartyB's position, which should not have been liquidated, gets liquidated instead, leading to a loss to PartyB because a certain portion of the position or its locked values need to be given to the liquidator as liquidation fees.
The liquidator (which is the PartyA triggering the force closing) will receive more liquidation fees than expected, which is incorrect.
There is also another possible scenario where an incorrect reserveAmount value after downcast at Line 118 causes the condition at Line 118 to be wrongly evaluated, leading to the quote being closed instead of PartyB being liquidated.
Following is the list of possible negative impacts due to unsafe casting, summarized from the above report:
PartyA cannot force close this position due to a revert
PartyB's position, which should not have been liquidated, gets liquidated instead, leading to a loss to PartyB because a certain portion of the position or its locked values need to be given to the liquidator as liquidation fees.
The liquidator (which is the PartyA triggering the force closing) will receive more liquidation fees than expected, which is incorrect.
Quote incorrectly being closed instead of PartyB being liquidated.
PoC
No response
Mitigation
Consider implementing the following measures to mitigate the issue:
xiaoming90
High
Unsafe casting of
reserveAmountfrom uint256 to int256Summary
Unsafe casting of
reserveAmountfrom uint256 to int256, leading to an incorrect value. As a result, it could lead to DOS or a loss of assets, as shown in the report below.Root Cause
reserveAmountfrom uint256 to int256, leading to an incorrect valueInternal pre-conditions
No response
External pre-conditions
No response
Attack Path
At Line 118 below, the code performs an unsafe downcasting from uint256 to int256. If the
reserveAmountexceeds the maximum value of anint256 (2^255 - 1), the conversion will not revert automatically. Instead, it will silently result in an incorrect wrapped value.Note: It is possible for the
reserveAmountto exceed2^255 - 1within the protocol as thedepositToReserveVaultfunction does not restrict users from depositing an amount larger than2^255 - 1andaccountLayout.reserveVault[partyB]is uint256.https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacetImpl.sol#L118
The following demonstrates a scenario where a
reserveAmountthat is larger thanint256 (2^255 - 1)resulting in the condition (partyBAvailableBalance + int256(reserveAmount)) at Line 118 above to be evaluated to be incorrect. WhenreserveAmountis a positive value of2**256 - 100, it will become a negative value (-100) when downcasted from uint256 to int256, as shown below.If there is no downcasting error, the condition at Line 118 should have been evaluated as
Truesince:However, as shown in the chisel output above, the final result is a negative number (
result = partyBAvailableBalance + reserveAmountDowncast = -250), which leads to the condition being incorrectly evaluated as false.This PartyB, with a large amount of reserve
(2**256 - 100), should allow any force close action against them to be executed smoothly (without reverting or being liquidated) by going into the second branch of the if-else branch (Code in Lines 118-126).However, due to the downcasting error, the logic in the final-else branch is executed instead (Lines 128-137 above), which contains the code for liquidating the PartyB (Line 127 above). Eventually, it is likely that the liquidation against PartyB will revert as the
availableBalancewithin theLibLiquidation.availableBalancefunction will likely be larger than 0. This also means that PartyA cannot force close this position due to this revert.On the other hand, it is possible that the liquidation process will go through without a revert (If the
availableBalancewithinliquidatePartyBfunction increases, but still remains as a negative value). In this case, the consequences are as follows:There is also another possible scenario where an incorrect
reserveAmountvalue after downcast at Line 118 causes the condition at Line 118 to be wrongly evaluated, leading to the quote being closed instead of PartyB being liquidated.https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacetImpl.sol#L118
Impact
Following is the list of possible negative impacts due to unsafe casting, summarized from the above report:
PoC
No response
Mitigation
Consider implementing the following measures to mitigate the issue: