slither icon indicating copy to clipboard operation
slither copied to clipboard

Enhancement: Add Balance Reliance Detector

Open dguido opened this issue 4 months ago • 1 comments

Summary

Add a new detector that identifies potentially unsafe uses of address.balance in smart contracts, specifically when used in strict equality comparisons or assigned to state variables.

Motivation

The use of address.balance in certain contexts can lead to vulnerabilities and unexpected behavior:

  1. Strict Equality Comparisons: Using address.balance == expectedValue or address.balance != expectedValue is inherently unreliable because:

    • An attacker can forcibly send ETH to any address using selfdestruct
    • Pre-calculated contract addresses can receive ETH before deployment
    • Balance can change between transactions in unpredictable ways
  2. State Variable Assignment: Storing balance values in state variables can lead to stale data and incorrect assumptions about current balances.

These patterns frequently lead to bugs where contracts make incorrect assumptions about ETH balances, potentially causing:

  • Denial of service attacks
  • Broken invariants
  • Failed assertions
  • Incorrect business logic

Proposed Implementation

The detector should:

  1. Track address.balance usage throughout the codebase

  2. Identify problematic patterns:

    • Use in binary operations with == or != operators
    • Assignment to state variables
    • Use in require/assert statements with strict equality
  3. Trace data flow to catch indirect usage:

    uint256 bal = address(this).balance;
    require(bal == expectedAmount); // Should be detected
    
  4. Report with appropriate severity:

    • Impact: WARNING
    • Confidence: LOW-MEDIUM (depending on context)

Example Vulnerable Code

contract VulnerableContract {
    uint256 public savedBalance;
    
    // BAD: Strict equality check
    function checkExactBalance() public view {
        require(address(this).balance == 1 ether, "Balance must be exactly 1 ETH");
    }
    
    // BAD: Saving balance to state
    function saveCurrentBalance() public {
        savedBalance = address(this).balance;
    }
    
    // BAD: Indirect strict comparison
    function indirectCheck(uint256 expected) public view {
        uint256 currentBal = address(this).balance;
        require(currentBal == expected, "Balance mismatch");
    }
}

Recommended Patterns

The detector should suggest alternatives:

contract SafeContract {
    // GOOD: Use >= or <= for balance checks
    function checkMinimumBalance() public view {
        require(address(this).balance >= 1 ether, "Insufficient balance");
    }
    
    // GOOD: Check balance ranges
    function checkBalanceRange() public view {
        require(
            address(this).balance >= 1 ether && 
            address(this).balance <= 10 ether,
            "Balance out of range"
        );
    }
}

Benefits

  1. Prevent common vulnerabilities related to balance manipulation
  2. Educational value for developers unfamiliar with ETH forcing attacks
  3. Reduce attack surface in DeFi protocols and payment systems
  4. Complement existing detectors for comprehensive security analysis

Priority

High - This is a common vulnerability pattern that can lead to significant issues in production contracts, especially in DeFi protocols where balance assumptions are critical for security.

dguido avatar Aug 29 '25 17:08 dguido

Analysis: Relationship to Existing incorrect-equality Detector

After reviewing the existing codebase, I found that Slither already has partial coverage for balance-related issues through the incorrect-equality detector (slither/detectors/statements/incorrect_strict_equality.py). However, the proposed enhancement would add valuable specificity and coverage.

Current Implementation

The existing detector already identifies some balance-related patterns:

def taint_balance_equalities(self, functions):
    taints = []
    for func in functions:
        for node in func.nodes:
            for ir in node.irs_ssa:
                # Detects address.balance
                if isinstance(ir, SolidityCall) and ir.function == SolidityFunction("balance(address)"):
                    taints.append(ir.lvalue)
                # Detects balanceOf() calls
                if isinstance(ir, HighLevelCall):
                    if isinstance(ir.function, Function) and ir.function.full_name == "balanceOf(address)":
                        taints.append(ir.lvalue)

It then flags these tainted values when used in strict equality (==) comparisons.

Value of Proposed Enhancement

The proposed Balance Reliance Detector would add significant value by:

  1. Broader Detection: Beyond just == comparisons, it would catch balance assignments to state variables, which can create persistent vulnerabilities
  2. Specificity: A dedicated detector would provide clearer guidance about the specific risk (forced ETH via selfdestruct)
  3. State Variable Focus: The current detector primarily focuses on local comparisons, while this would emphasize state-level reliance

Implementation Options

  1. Enhance Existing Detector: Add balance-to-state-variable assignment detection to incorrect-equality
  2. Create Sub-Detector: Keep as separate but share taint analysis infrastructure
  3. Split by Severity: State variable assignments (HIGH severity) vs local comparisons (MEDIUM severity)

Example Detection Pattern

For the proposed detector, we'd want to catch patterns like:

# Detect balance assignments to state variables
if isinstance(ir, Assignment):
    if ir.lvalue in contract.state_variables:
        if self.is_balance_related(ir.rvalue):
            # Flag: State variable relies on balance

This enhancement would strengthen Slither's ability to detect the full spectrum of balance reliance vulnerabilities, complementing the existing strict equality checks with broader state management analysis.

dguido avatar Aug 29 '25 18:08 dguido