code-423n4 / redacted-bug-bounty

13 stars 9 forks source link

integer overflow attack #64

Closed c4-bot-6 closed 4 months ago

c4-bot-6 commented 4 months ago

Lines of code

https://github.com/redacted-cartel/pirex-eth-contracts/blob/11f30c7e35b67d45deefe405c22a30f352bc5b21/src/PirexEth.sol#L460

Vulnerability details

I have identified a potential vulnerability related to the feeAmount calculation in the instantRedeemWithPxEth function.

Vulnerability:

The instantRedeemWithPxEth function calculates the feeAmount based on the assets parameter, which is provided by the user. This calculation does not include any upper limit, making it susceptible to an integer overflow attack. An attacker can exploit this vulnerability to create a large feeAmount, leading to an overflow, and eventually, the postFeeAmount can become zero, allowing the attacker to redeem all the pxETH without paying any fees.

Impact:

An attacker can exploit this vulnerability to drain the pxETH balance from the contract without paying the required fees, resulting in potential loss of funds.

Exploitation:

An attacker can create a malicious transaction with an extremely large assets value to trigger an integer overflow in the feeAmount calculation. This will lead to a zero or near-zero postFeeAmount, allowing the attacker to redeem all the pxETH without paying any fees.

Recommendation:

To mitigate this vulnerability, implement a maximum limit on the feeAmount based on the assets parameter. This can be done by either using the maxFees mapping to check for any predefined maximum fees or by calculating the maximum allowed fee amount based on the remaining pxETH in the contract.

Here is an example of how to implement the fix:

  1. Update the instantRedeemWithPxEth function to calculate the maxFeeAmount:
uint256 maxFeeAmount = (totalPxEth * maxFees[DataTypes.Fees.InstantRedemption]) / DENOMINATOR;
  1. Implement a check to ensure the feeAmount is less than or equal to the maxFeeAmount before proceeding with the redemption process:
require(feeAmount <= maxFeeAmount, "Error: Fee amount exceeds the maximum limit.");

By implementing this fix, you can prevent the integer overflow attack and secure the smart contract against potential fee manipulation.

c4-bot-2 commented 4 months ago

Discord id(s) for hunter(s): [object Object]