Submission Details
Impact:
Likelihood:
Potential Integer Overflow in Swap Amount Calculation
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
While the current implementation handles negative `amountSpecified` values correctly, there's a theoretical edge case with very large values, though this is mitigated by the pool's own validation.
**Description**:
* The normal behavior expects `amountSpecified` to be converted to a positive uint256 safely for fee calculations.
* The specific issue is that converting a very large negative int256 to uint256 could theoretically cause issues, though in practice, the pool manager validates swap amounts before calling the hook, making this unlikely.
**Root cause in the codebase**:
```solidity
// @> src/RebateFiHook.sol:155-157
uint256 swapAmount = params.amountSpecified < 0
? uint256(-params.amountSpecified)
: uint256(params.amountSpecified);
```
If `amountSpecified` is `type(int256).min` (-2^255), then `-amountSpecified` would overflow int256. However, this is unlikely as the pool validates amounts.
Risk
Likelihood:
-
* This would require `amountSpecified` to be exactly `type(int256).min`, which is extremely unlikely
* The pool manager validates swap amounts before calling hooks, preventing this scenario
Impact:
-
* If it occurred, the conversion would revert, preventing the swap
* The swap would fail with an overflow error
Proof of Concept
The following demonstrates the theoretical edge case (though unlikely in practice):
```solidity
// Edge case: amountSpecified is exactly type(int256).min
// type(int256).min = -2^255 = -57896044618658097711785492504343953926634992332820282019728792003956564819968
int256 amountSpecified = type(int256).min; // Minimum possible int256 value
// Current implementation in _beforeSwap():
uint256 swapAmount = params.amountSpecified < 0
? uint256(-params.amountSpecified) // Attempting to negate type(int256).min
: uint256(params.amountSpecified);
// Problem:
// -amountSpecified = -type(int256).min = 2^255
// But int256.max = 2^255 - 1
// So -type(int256).min cannot be represented as int256 (would overflow)
// Converting to uint256: uint256(2^255) = 2^255, which is valid
// However, the negation operation itself would overflow int256
// In Solidity 0.8+, arithmetic overflow causes revert:
// -type(int256).min would cause: "Arithmetic over/underflow"
// Transaction would revert before reaching uint256 conversion
// However, this scenario is prevented because:
// 1. Pool manager validates swap amounts before calling hooks
// 2. No realistic swap would use type(int256).min as amount
// 3. Amounts are typically much smaller (e.g., 1e18 wei)
```
**Step-by-step execution (theoretical):**
1. User somehow provides `amountSpecified = type(int256).min` (-2^255)
2. Pool manager would validate this before calling hook (prevents scenario)
3. If it reached the hook, code checks: `params.amountSpecified < 0` → true
4. Attempts: `uint256(-params.amountSpecified)`
5. Negation: `-type(int256).min` = 2^255 (exceeds int256.max = 2^255 - 1)
6. Solidity 0.8+ detects overflow, reverts with arithmetic error
7. Swap fails, but this is defensive behavior
**Note:** This is extremely unlikely as the pool manager validates amounts before calling hooks, making this a theoretical edge case rather than a practical vulnerability.
Recommended Mitigation
Add explicit check for the edge case before negation:
```diff
// src/RebateFiHook.sol:48-50
+ error InvalidAmountSpecified();
// src/RebateFiHook.sol:155-157
- uint256 swapAmount = params.amountSpecified < 0
- ? uint256(-params.amountSpecified)
- : uint256(params.amountSpecified);
+ uint256 swapAmount;
+ if (params.amountSpecified < 0) {
+ // Check for edge case: type(int256).min cannot be negated safely
+ if (params.amountSpecified == type(int256).min) {
+ revert InvalidAmountSpecified();
+ }
+ swapAmount = uint256(-params.amountSpecified);
+ } else {
+ swapAmount = uint256(params.amountSpecified);
+ }
```
**Explanation:**
- Explicitly check for `type(int256).min` before attempting negation
- Prevents potential arithmetic overflow during negation
- Provides clear error message if edge case occurs
- Defensive programming practice, though pool manager validation likely prevents this
- Follows principle of explicit validation over implicit behavior
- Note: This is a low-priority fix as the scenario is extremely unlikely in practice
Rewards breakdown
nSLOC
150This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.