Skip to content

Latest commit

 

History

History
50 lines (31 loc) · 1.78 KB

File metadata and controls

50 lines (31 loc) · 1.78 KB

Fit Menthol Sawfish

Medium

Users will be DoSed from swapping if they specify the real maximum amount to swap

Summary

TradingLimits::update() calculates the new netflow of the asset by using TradingLimits::safeINT48Add(). In the latter, it ensures that the result would not overflow an int48 number, but the lower limit is incorrectly defined as -1 * MAX_INT48, as in reality int48 numbers can be as small as -1 * MAX_INT48 - 1.

The swap reverts because the user may infer that there is type(int48).min - self.netflow left and swap this amount, but in reality there is only type(int48).min - 1 - self.netflow left, which reverts.

Root Cause

In TradingLimits:169, the lower limit is incorrectly defined as -1 * MAX_INT48, which is 1 unit bigger than the real limit, type(int48).min.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. User swaps via Broker::swapIn/Out() with the real maximum amount possible but it reverts because the lower limit is bigger than it should.

Impact

Swap is DoSed due to the lower limit bug which is time sensitive as the user may get a worse price when it tries again.

PoC

function test_POC_tradingLimitsMin() public pure { console.log(type(int48).min)); console.log(-1 * type(int48).max)); }

Mitigation

MIN_INT48 = type(int48).min;
...
function safeINT48Add(int48 a, int48 b) internal pure returns (int48) {
  int256 c = int256(a) + int256(b);
  require(c >= MIN_INT48 && c <= MAX_INT48, "int48 addition overflow");
  return int48(c);
}