crytic / crytic/slither

Enhancement: Add Balance Reliance Detector

Open
#2,778 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
6.4k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

## 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:
```solidity
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

```solidity
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:

```solidity
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.