Remediating Business Logic Errors

Overview

Related Detector: Business Logic Error

Business logic errors occur when smart contract code correctly executes its instructions but those instructions encode incorrect protocol rules. Remediation requires enforcing protocol invariants explicitly, ordering state updates correctly, validating edge cases, and using established standards where available.

Recommended Fix

Before (Vulnerable)

// Reward calculation updates timestamp after transfer — double-claim risk
contract VulnerableRewards {
    mapping(address => uint256) public stakes;
    mapping(address => uint256) public lastClaim;
    IERC20 public rewardToken;
    uint256 constant RATE = 1e15; // per second per staked token

    function claim() external {
        uint256 pending = calculatePending(msg.sender);
        require(pending > 0, "Nothing to claim");

        // VULNERABLE: external call before accounting update
        rewardToken.transfer(msg.sender, pending);  // callback possible here
        lastClaim[msg.sender] = block.timestamp;    // too late
    }

    function calculatePending(address user) public view returns (uint256) {
        return stakes[user] * RATE * (block.timestamp - lastClaim[user]);
    }
}

After (Fixed)

// Fixed: accounting update precedes external transfer
contract SecureRewards {
    mapping(address => uint256) public stakes;
    mapping(address => uint256) public lastClaim;
    IERC20 public rewardToken;
    uint256 constant RATE = 1e15;

    function claim() external {
        uint256 pending = calculatePending(msg.sender);
        require(pending > 0, "Nothing to claim");

        // FIXED: update accounting state BEFORE transfer
        lastClaim[msg.sender] = block.timestamp;
        rewardToken.transfer(msg.sender, pending);
    }

    function calculatePending(address user) public view returns (uint256) {
        return stakes[user] * RATE * (block.timestamp - lastClaim[user]);
    }
}

The fix applies the Checks-Effects-Interactions pattern: update all state (Effects) before making external calls (Interactions). This ensures that even if the recipient re-enters the claim function, the lastClaim timestamp is already updated and the pending amount calculates to zero.

Alternative Mitigations

Add Explicit Invariant Checks

Assert invariants at the end of critical functions:

function deposit(uint256 amount) external {
    uint256 sharesBefore = totalShares;
    uint256 assetsBefore = totalAssets();

    token.transferFrom(msg.sender, address(this), amount);
    uint256 newShares = calculateShares(amount);
    totalShares += newShares;
    _mint(msg.sender, newShares);

    // Assert invariant: share price must not decrease after deposit
    assert(totalAssets() * 1e18 / totalShares >= assetsBefore * 1e18 / sharesBefore);
}

Use Minimum Amounts to Prevent Rounding Exploits

uint256 constant MIN_DEPOSIT = 1e6;  // 1 USDC minimum

function deposit(uint256 amount) external {
    require(amount >= MIN_DEPOSIT, "Below minimum");
    uint256 fee = (amount * FEE_BPS) / 10000;
    require(fee > 0, "Fee rounds to zero — amount too small");
    // Proceed with deposit
}

Follow ERC-4626 for Vaults

ERC-4626 specifies the correct ordering for vault deposit and withdrawal operations. Adopt its previewDeposit / previewWithdraw model:

// ERC-4626 compliant: preview uses CURRENT state before mutation
function deposit(uint256 assets, address receiver) public returns (uint256 shares) {
    shares = previewDeposit(assets);      // calculate with current state
    _deposit(msg.sender, receiver, assets, shares);  // transfer + mint
}

Common Mistakes

Mistake: Calculating Shares After Transfer

// WRONG: calculates shares using post-transfer total assets
function deposit(uint256 amount) external {
    token.transferFrom(msg.sender, address(this), amount);  // transfers first
    // Now totalAssets() includes the new amount — calculation is wrong
    uint256 shares = (amount * totalShares) / totalAssets();
    _mint(msg.sender, shares);
}

// CORRECT: calculate before transfer
function deposit(uint256 amount) external {
    uint256 shares = totalShares == 0
        ? amount
        : (amount * totalShares) / totalAssets();  // uses pre-transfer state
    token.transferFrom(msg.sender, address(this), amount);
    _mint(msg.sender, shares);
}

Mistake: Unguarded Admin Functions With No Invariant Checks

// WRONG: admin can set arbitrary parameters without bounds checking
function setRewardRate(uint256 newRate) external onlyOwner {
    rewardRate = newRate;  // No validation — could set to astronomically high value
}

// CORRECT: validate parameters maintain protocol invariants
function setRewardRate(uint256 newRate) external onlyOwner {
    require(newRate <= MAX_REWARD_RATE, "Exceeds maximum allowed rate");
    require(newRate >= MIN_REWARD_RATE, "Below minimum viable rate");
    rewardRate = newRate;
    emit RewardRateUpdated(newRate);
}