Uniswap V2 (partial review)

Description

The swap function triggers a callback to the recipient before state updates complete. A malicious pair contract or receiver can re-enter and drain liquidity during the callback window.

Recommendation

Apply checks-effects-interactions pattern. Update all state (balances, reserves) before performing the callback. Consider ReentrancyGuard as defense-in-depth.

Code

// Vulnerable: callback before state finalization
function swap(uint amount0Out, uint amount1Out, address to, bytes data) external {
    // ... checks ...
    if (amount0Out > 0) _safeTransfer(_token0, to, amount0Out);
    if (amount1Out > 0) _safeTransfer(_token1, to, amount1Out);
    if (data.length > 0) IUniswapV2Callee(to).uniswapV2Call(...);  // Re-entry point
    _update(balance0, balance1, ...);  // State update too late
}

Description

An attacker can flash loan, call sync() to skew reserves, execute arbitrage, and repay within the same transaction. While not a direct exploit of Pair logic, it enables low-cost MEV extraction.

Recommendation

Document sync() behavior. Consider time-weighted oracles for dependent protocols. No change required in Pair—informational for integrators.

Code

function sync() external lock {
    _update(IERC20(token0).balanceOf(address(this)),
            IERC20(token1).balanceOf(address(this)),
            reserve0, reserve1);
}

Description

Integer division in getAmountOut can favor the pool in certain edge cases. With small amounts, the rounding may consistently round down for users.

Recommendation

Document expected rounding behavior. For protocols requiring exact amounts, use getAmountIn or add safety margins.

Code

function getAmountOut(uint amountIn, uint reserveIn, uint reserveOut)
    public pure returns (uint amountOut) {
    require(amountIn > 0 && reserveIn > 0 && reserveOut > 0);
    uint amountInWithFee = amountIn * 997;
    uint numerator = amountInWithFee * reserveOut;
    uint denominator = reserveIn * 1000 + amountInWithFee;
    amountOut = numerator / denominator;  // Truncation
}

Description

addLiquidity and related Router functions do not explicitly reject tokenA == tokenB or token == address(0). Can lead to accidental loss of funds.

Recommendation

Add require(tokenA != tokenB) and require(tokenA != address(0) && tokenB != address(0)) at Router entry points.

Code

function addLiquidity(
    address tokenA,
    address tokenB,
    uint amountADesired,
    uint amountBDesired,
    // ... no zero / same-token checks
) public returns (uint amountA, uint amountB, uint liquidity) { ... }