<!-- canonical: https://0xsimao.com/findings/mento-limits-delta-flow-becomes -->

# `TradingLimits::update()` incorrectly only rounds up when `deltaFlowUnits` becomes 0, which will silently increase trading limits

Medium · Sherlock · Stablecoin · 24th October 2024

Finding M-4 of the Mento competition.

- Protocol: https://audits.sherlock.xyz/contests/598
- Report: /reports/mento
- Codebase: https://github.com/0xsimao/2024-10-mento-update/tree/098b17fb32d294145a7f000d96917d13db8756cc
- Source: https://github.com/sherlock-audit/2024-10-mento-update-judging/issues/45

---

### Summary

[TradingLimits::update()](https://github.com/sherlock-audit/2024-10-mento-update/blob/main/mento-core/contracts/libraries/TradingLimits.sol#L124) divides the traded funds by the decimals of the token, int256 _deltaFlowUnits = _deltaFlow / int256((10 ** uint256(decimals)));`. In a token with 18 decimals, for example, swapping 1.999...e18 tokens will lead to a `_deltaFlowUnits` of just `1`, taking a major error. This can be exploited to swap up to twice the trading limit, if tokens are swapped 2 by 2 and the state is updated only by 1 each time. Overall, even without malicious intent, the limits will always be bypassed due to the rounding.

### Root Cause

In `TradingLimits:135`, it only rounds up whenever `deltaFlowUnits` becomes 0, but the error is just as big if it becomes 1 from 2, effectively not providing enough protection.

### Internal pre-conditions

None.

### External pre-conditions

None.

### Attack Path

1. User calls `Broker::swapIn/Out()` with amounts in and out that produce rounding errors (almost always).

### Impact

The trading limits may be severely bypassed with malicious intent (by double the amount) or by a smaller but still significant amount organically.

### PoC

`TradingLimits::update()` only rounds up when `deltaFlowUnits` becomes 0.
```solidity
function update(
  ITradingLimits.State memory self,
  ITradingLimits.Config memory config,
  int256 _deltaFlow,
  uint8 decimals
) internal view returns (ITradingLimits.State memory) {
  int256 _deltaFlowUnits = _deltaFlow / int256((10 ** uint256(decimals)));
  require(_deltaFlowUnits <= MAX_INT48, "dFlow too large");
  
  int48 deltaFlowUnits = int48(_deltaFlowUnits);
  if (deltaFlowUnits == 0) {
    deltaFlowUnits = _deltaFlow > 0 ? int48(1) : int48(-1);
  }
  ...
```

### Mitigation

The correct fix is:
```solidity
int256 _deltaFlowUnits = (_deltaFlow - 1) / int256((10 ** uint256(decimals))) + 1;
```

---

Related findings:

- [`fyToken` contribution limits are incorrect as they compare `fyToken` and `buyToken` amounts with `idoSize` in `idoToken` units](https://0xsimao.com/findings/blast-ido-pools-contribution-limits-compare-buy): Blast IDO Pools
- [An attacker may DoS user Fluid balance increases by frontrunning `FluidLocker::claim()` calls and calling `EP_PROGRAM_MANAGER::batchUpdateUserUnits()` directly](https://0xsimao.com/findings/superfluid-locker-system-increases-frontrunning-program-directly): Superfluid Locker System
- [OstiumTrading::updateOpenLimitOrder() Makes Unnecessary Field Updates](https://0xsimao.com/findings/ostium-trading-unnecessary-field-updates): Ostium