Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions slither/detectors/statements/incorrect_strict_equality.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
)

from slither.core.solidity_types import MappingType, ElementaryType
from slither.core.solidity_types.elementary_type import MaxValues

from slither.core.variables.state_variable import StateVariable
from slither.core.declarations.solidity_variables import (
Expand Down Expand Up @@ -87,6 +88,24 @@ def is_not_comparing_addresses(ir: Binary) -> bool:

return True

@staticmethod
def is_comparing_against_safe_constant(ir: Binary) -> bool:
"""
Returns True if the comparison is against a safe boundary constant
that cannot be manipulated by an attacker (0, type(T).max, etc.)
These comparisons are common patterns and should not be flagged.
Fixes: https://github.com/crytic/slither/issues/2759
"""
for var in [ir.variable_left, ir.variable_right]:
if isinstance(var, Constant):
# Check for zero - comparing against 0 is safe
if var.value == 0:
return True
# Check for type(T).max values (e.g., type(uint256).max)
if var.value in MaxValues.values():
return True
return False

@staticmethod
def is_any_tainted(
variables: list[
Expand Down Expand Up @@ -155,6 +174,8 @@ def tainted_equality_nodes(
self.is_direct_comparison(ir)
# Filter out address comparisons which may occur due to lack of field sensitivity in data dependency
and self.is_not_comparing_addresses(ir)
# Filter out comparisons against safe boundary constants (0, type(T).max)
and not self.is_comparing_against_safe_constant(ir)
and self.is_any_tainted(ir.used, taints, func)
):
if func not in results:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,12 @@ TestContractBalance.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.4
ERC20TestBalance.bad1(ERC20Variable) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#25-27) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(msg.sender) == 10) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#26)

TestSolidityKeyword.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#123-125) uses a dangerous strict equality:
- require(bool)(now == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#124)

TestContractBalance.bad4() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#52-57) uses a dangerous strict equality:
- balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#54)

ERC20TestBalance.bad0(ERC20Function) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#21-23) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(address(this)) == 10) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#22)

TestSolidityKeyword.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#127-129) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#128)

TestContractBalance.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#32-35) uses a dangerous strict equality:
- require(bool)(address(address(this)).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#33)

Expand All @@ -28,9 +22,6 @@ TestContractBalance.bad5() (tests/e2e/detectors/test_data/incorrect-equality/0.4
TestContractBalance.bad6() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#66-71) uses a dangerous strict equality:
- balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#68)

TestSolidityKeyword.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#131-133) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#132)

TestContractBalance.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#42-45) uses a dangerous strict equality:
- require(bool)(address(this).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.4.25/incorrect_equality.sol#43)

Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,9 @@ TestContractBalance.bad5() (tests/e2e/detectors/test_data/incorrect-equality/0.5
TestContractBalance.bad4() (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#52-57) uses a dangerous strict equality:
- balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#54)

TestSolidityKeyword.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#127-129) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#128)

TestContractBalance.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#42-45) uses a dangerous strict equality:
- require(bool)(address(this).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#43)

ERC20TestBalance.bad0(ERC20Function) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#21-23) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(address(this)) == 10) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#22)

TestSolidityKeyword.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#123-125) uses a dangerous strict equality:
- require(bool)(now == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#124)

TestSolidityKeyword.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#131-133) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.5.16/incorrect_equality.sol#132)

Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,18 @@ TestContractBalance.bad3() (tests/e2e/detectors/test_data/incorrect-equality/0.6
TestContractBalance.bad6() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#66-71) uses a dangerous strict equality:
- balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#68)

TestSolidityKeyword.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#123-125) uses a dangerous strict equality:
- require(bool)(now == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#124)

TestContractBalance.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#42-45) uses a dangerous strict equality:
- require(bool)(address(this).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#43)

ERC20TestBalance.bad1(ERC20Variable) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#25-27) uses a dangerous strict equality:
- require(bool)(erc.balanceOf(msg.sender) == 10) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#26)

TestSolidityKeyword.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#127-129) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#128)

TestContractBalance.bad4() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#52-57) uses a dangerous strict equality:
- balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#54)

TestContractBalance.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#37-40) uses a dangerous strict equality:
- require(bool)(10000000000000000000 == address(address(this)).balance) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#38)

TestSolidityKeyword.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#131-133) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#132)

TestContractBalance.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#32-35) uses a dangerous strict equality:
- require(bool)(address(address(this)).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.6.11/incorrect_equality.sol#33)

Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ ERC20TestBalance.bad0(ERC20Function) (tests/e2e/detectors/test_data/incorrect-eq
TestContractBalance.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#37-40) uses a dangerous strict equality:
- require(bool)(10000000000000000000 == address(address(this)).balance) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#38)

TestSolidityKeyword.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#123-125) uses a dangerous strict equality:
- require(bool)(block.timestamp == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#124)
TestSafeConstants.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#166-168) uses a dangerous strict equality:
- address(this).balance == 10000000000000000000 (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#167)

TestContractBalance.bad3() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#47-50) uses a dangerous strict equality:
- require(bool)(10000000000000000000 == address(this).balance) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#48)
Expand All @@ -22,15 +22,15 @@ TestContractBalance.bad4() (tests/e2e/detectors/test_data/incorrect-equality/0.7
TestContractBalance.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#32-35) uses a dangerous strict equality:
- require(bool)(address(address(this)).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#33)

TestSolidityKeyword.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#131-133) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#132)

TestContractBalance.bad5() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#59-64) uses a dangerous strict equality:
- 10000000000000000000 == balance (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#61)

TestSolidityKeyword.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#136-138) uses a dangerous strict equality:
- require(bool)(block.number == 100) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#137)

TestContractBalance.bad2() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#42-45) uses a dangerous strict equality:
- require(bool)(address(this).balance == 10000000000000000000) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#43)

TestSolidityKeyword.bad1() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#127-129) uses a dangerous strict equality:
- require(bool)(block.number == 0) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#128)
TestSolidityKeyword.bad0() (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#132-134) uses a dangerous strict equality:
- require(bool)(block.timestamp == 1) (tests/e2e/detectors/test_data/incorrect-equality/0.7.6/incorrect_equality.sol#133)

Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,52 @@ contract TestSolidityKeyword{
}
}

// Comparing against 0 is safe - cannot be manipulated
function good4() external{
require(block.timestamp == 0);
}

function good5() external{
require(block.number == 0);
}

function bad0() external{
require(block.timestamp == 0);
require(block.timestamp == 1);
}

function bad1() external{
require(block.number== 0);
require(block.number == 100);
}

function bad2() external{
require(block.number == 0);
}

contract TestSafeConstants {
mapping(address => uint256) public balances;

// Comparing balance against 0 is safe - cannot reach 0 by adding value
function good0() external view returns (bool) {
return balances[msg.sender] == 0;
}

// Comparing against type(uint256).max is safe - common infinite approval pattern
function good1() external view returns (bool) {
return balances[msg.sender] == type(uint256).max;
}

// Comparing ETH balance against 0 is safe
function good2() external view returns (bool) {
return address(this).balance == 0;
}

// Comparing against arbitrary value is NOT safe
function bad0() external view returns (bool) {
return balances[msg.sender] == 100;
}

// Comparing against arbitrary ETH value is NOT safe
function bad1() external view returns (bool) {
return address(this).balance == 10 ether;
}
}

interface Receiver {
Expand Down
Binary file not shown.