Add ERC20 approval race audit check

README.md

@@ -173,6 +173,7 @@ or open [`web/index.html`](web/index.html) directly.
 | `OracleCheck` | Spot price reads (manipulable), missing TWAP, single-source oracles |
 | `UpgradeCheck` | Storage layout collisions in proxies, uninitialized implementation contracts |
 | `FlashLoanCheck` | Functions vulnerable to flash-loan-powered price/state manipulation |
+| `ERC20ApprovalRaceCheck` | Unsafe ERC-20 allowance overwrites that enable approve front-running |
 
 ## Architecture
 
@@ -192,6 +193,10 @@ test/
 └── Example.t.sol            — Full example audit against VulnerableVault
 ```
 
+The ERC-20 approval race check includes `src/checks/ERC20ApprovalRaceCheck.sol`,
+`src/examples/VulnerableERC20Approval.sol`, `test/ERC20ApprovalRace.t.sol`, and
+the companion Slither heuristic in `slither/detectors/custom_erc20_approval_race.py`.
+
 ## License
 
 MIT

slither/detectors/custom_erc20_approval_race.py

@@ -0,0 +1,103 @@
+"""Custom Slither detector for ERC-20 approval race hazards.
+
+Detects token-like contracts whose approve(address,uint256) implementation
+directly overwrites allowance state without an obvious zero-first or
+delta-based allowance guard. This flags the classic allowance front-running
+pattern where a spender can consume the old allowance before a lower approval
+lands, then consume the new allowance as well.
+"""
+try:
+    from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+except ModuleNotFoundError:
+    class DetectorClassification:
+        MEDIUM = "medium"
+
+    class AbstractDetector:
+        pass
+
+
+ALLOWANCE_NAMES = ("allowance", "allowances", "_allowances")
+SAFE_GUARD_TERMS = (
+    "increaseallowance",
+    "decreaseallowance",
+    "safeincreaseallowance",
+    "safedecreaseallowance",
+)
+
+
+def _name_of(item) -> str:
+    return str(getattr(item, "name", item)).lower()
+
+
+def is_approve_function(function) -> bool:
+    if _name_of(function) != "approve":
+        return False
+    parameters = list(getattr(function, "parameters", []) or [])
+    return len(parameters) == 2
+
+
+def function_writes_allowance(function) -> bool:
+    for node in getattr(function, "nodes", []) or []:
+        for ir in getattr(node, "irs", []) or []:
+            writes = getattr(ir, "writes", None) or []
+            for write in writes:
+                variable = write[0] if isinstance(write, tuple) and write else write
+                if _name_of(variable) in ALLOWANCE_NAMES:
+                    return True
+    text = str(getattr(function, "source_mapping", "")) + " " + str(getattr(function, "nodes", ""))
+    lowered = text.lower()
+    return any(name in lowered for name in ALLOWANCE_NAMES)
+
+
+def has_allowance_race_guard(function) -> bool:
+    text = " ".join(str(item).lower() for item in getattr(function, "nodes", []) or [])
+    if any(term in text for term in SAFE_GUARD_TERMS):
+        return True
+    zero_guard_patterns = (
+        "amount == 0",
+        "amount==0",
+        "value == 0",
+        "value==0",
+        "current == 0",
+        "current==0",
+        "allowance[msg.sender][spender] == 0",
+        "_allowances[msg.sender][spender] == 0",
+    )
+    return any(pattern in text for pattern in zero_guard_patterns)
+
+
+def find_approval_race_functions(contract) -> list:
+    if getattr(contract, "is_interface", False):
+        return []
+
+    findings = []
+    for function in getattr(contract, "functions", []) or []:
+        if not getattr(function, "is_implemented", True):
+            continue
+        if is_approve_function(function) and function_writes_allowance(function) and not has_allowance_race_guard(function):
+            findings.append(function)
+    return findings
+
+
+class CustomERC20ApprovalRaceDetector(AbstractDetector):
+    ARGUMENT = "custom-erc20-approval-race"
+    HELP = "Detects ERC-20 approve implementations vulnerable to allowance race conditions"
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/kcolbchain/audit-checklist/wiki/ERC20-Approval-Race"
+    WIKI_TITLE = "Custom ERC20 Approval Race Detector"
+
+    def _detect(self) -> list:
+        results = []
+        for contract in self.compilation_unit.contracts:
+            for function in find_approval_race_functions(contract):
+                info = [
+                    "ERC20 approve may overwrite allowance unsafely: ",
+                    f"{function.name}() in {contract.name}\n",
+                    "\nPrefer increase/decrease allowance flows or reject direct non-zero allowance changes.\n",
+                ]
+                res = self.generate_result(info)
+                res.add(function)
+                results.append(res)
+        return results

slither/tests/test_custom_erc20_approval_race.py

@@ -0,0 +1,76 @@
+import importlib.util
+import pathlib
+import types
+import unittest
+
+
+ROOT = pathlib.Path(__file__).resolve().parents[2]
+DETECTOR_PATH = ROOT / "slither" / "detectors" / "custom_erc20_approval_race.py"
+
+spec = importlib.util.spec_from_file_location("custom_erc20_approval_race", DETECTOR_PATH)
+custom_erc20_approval_race = importlib.util.module_from_spec(spec)
+assert spec.loader is not None
+spec.loader.exec_module(custom_erc20_approval_race)
+
+
+def ir_writing(name):
+    return types.SimpleNamespace(writes=[types.SimpleNamespace(name=name)])
+
+
+def node(text, writes=None):
+    return types.SimpleNamespace(irs=writes or [], __str__=lambda self: text)
+
+
+class TextNode:
+    def __init__(self, text, writes=None):
+        self.text = text
+        self.irs = writes or []
+
+    def __str__(self):
+        return self.text
+
+
+def function(name, nodes):
+    return types.SimpleNamespace(
+        name=name,
+        parameters=[types.SimpleNamespace(name="spender"), types.SimpleNamespace(name="amount")],
+        nodes=nodes,
+        is_implemented=True,
+    )
+
+
+def contract(functions):
+    return types.SimpleNamespace(
+        name="Token",
+        functions=functions,
+        is_interface=False,
+    )
+
+
+class CustomERC20ApprovalRaceDetectorTest(unittest.TestCase):
+    def test_flags_approve_that_writes_allowance_without_guard(self):
+        approve = function("approve", [TextNode("allowance[msg.sender][spender] = amount", [ir_writing("allowance")])])
+
+        findings = custom_erc20_approval_race.find_approval_race_functions(contract([approve]))
+
+        self.assertEqual(findings, [approve])
+
+    def test_ignores_approve_with_zero_guard(self):
+        approve = function(
+            "approve",
+            [
+                TextNode("require(current == 0 || amount == 0)"),
+                TextNode("allowance[msg.sender][spender] = amount", [ir_writing("allowance")]),
+            ],
+        )
+
+        self.assertEqual(custom_erc20_approval_race.find_approval_race_functions(contract([approve])), [])
+
+    def test_ignores_non_approve_function(self):
+        transfer = function("transferFrom", [TextNode("allowance[from][msg.sender] -= amount", [ir_writing("allowance")])])
+
+        self.assertEqual(custom_erc20_approval_race.find_approval_race_functions(contract([transfer])), [])
+
+
+if __name__ == "__main__":
+    unittest.main()

src/checks/ERC20ApprovalRaceCheck.sol

@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
+
+import "../ChecklistBase.sol";
+
+interface IERC20ApprovalRaceTarget {
+    function approve(address spender, uint256 amount) external returns (bool);
+    function transferFrom(address from, address to, uint256 amount) external returns (bool);
+    function allowance(address owner, address spender) external view returns (uint256);
+    function balanceOf(address account) external view returns (uint256);
+}
+
+/// @title ERC20ApprovalRaceCheck - detect unsafe non-zero allowance overwrites
+/// @notice Models the classic ERC-20 approve race: an owner tries to lower a
+///         non-zero allowance, the spender uses the old allowance first, and
+///         then receives the new allowance as well. Tokens should prefer
+///         increase/decrease allowance flows or reject direct non-zero to
+///         non-zero approval changes.
+/// @author kcolbchain
+abstract contract ERC20ApprovalRaceCheck is ChecklistBase {
+    function getApprovalOwner() internal view virtual returns (address) {
+        return address(0xA11CE);
+    }
+
+    function getApprovalSpender() internal view virtual returns (address) {
+        return address(0xB0B);
+    }
+
+    function getApprovalRecipient() internal view virtual returns (address) {
+        return address(0xCAFE);
+    }
+
+    function getInitialApprovalAmount() internal view virtual returns (uint256) {
+        return 100 ether;
+    }
+
+    function getReducedApprovalAmount() internal view virtual returns (uint256) {
+        return 50 ether;
+    }
+
+    /// @dev Seed enough owner balance for the front-run old allowance spend
+    ///      and the later reduced allowance spend.
+    function seedOwnerBalance(address owner, uint256 amount) internal virtual;
+
+    function test_erc20_approval_change_cannot_be_double_spent() public {
+        IERC20ApprovalRaceTarget token = IERC20ApprovalRaceTarget(targetContract);
+        address owner = getApprovalOwner();
+        address spender = getApprovalSpender();
+        address recipient = getApprovalRecipient();
+        uint256 initialAmount = getInitialApprovalAmount();
+        uint256 reducedAmount = getReducedApprovalAmount();
+
+        require(initialAmount > reducedAmount, "initial must exceed reduced");
+        seedOwnerBalance(owner, initialAmount + reducedAmount);
+
+        vm.prank(owner);
+        bool initialApproved = token.approve(spender, initialAmount);
+        assertTrue(initialApproved, "initial approve failed");
+
+        vm.prank(spender);
+        bool oldAllowanceSpent = token.transferFrom(owner, recipient, initialAmount);
+        assertTrue(oldAllowanceSpent, "spender could not spend initial allowance");
+
+        vm.prank(owner);
+        try token.approve(spender, reducedAmount) returns (bool reducedApproved) {
+            if (!reducedApproved) {
+                return;
+            }
+        } catch {
+            return;
+        }
+
+        uint256 recipientBefore = token.balanceOf(recipient);
+        vm.prank(spender);
+        try token.transferFrom(owner, recipient, reducedAmount) returns (bool spentReduced) {
+            if (spentReduced && token.balanceOf(recipient) > recipientBefore) {
+                emit log_named_uint(
+                    "VULNERABILITY: spender extracted extra allowance after approval race",
+                    reducedAmount
+                );
+                assertLe(token.balanceOf(recipient), recipientBefore, "approval race allowed double spend");
+            }
+        } catch {
+            return;
+        }
+    }
+}

src/examples/VulnerableERC20Approval.sol

@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
+
+/// @title VulnerableERC20Approval - ERC-20 fixture with unsafe approve overwrite
+/// @notice DO NOT USE IN PRODUCTION. This token intentionally allows direct
+///         non-zero to non-zero allowance changes, which demonstrates the
+///         classic ERC-20 approval race.
+/// @author kcolbchain
+contract VulnerableERC20Approval {
+    string public name = "Vulnerable Approval Token";
+    string public symbol = "VAT";
+    uint8 public constant decimals = 18;
+
+    mapping(address => uint256) public balanceOf;
+    mapping(address => mapping(address => uint256)) public allowance;
+
+    function mint(address to, uint256 amount) external {
+        balanceOf[to] += amount;
+    }
+
+    function approve(address spender, uint256 amount) public virtual returns (bool) {
+        allowance[msg.sender][spender] = amount;
+        return true;
+    }
+
+    function transferFrom(address from, address to, uint256 amount) external returns (bool) {
+        uint256 allowed = allowance[from][msg.sender];
+        require(allowed >= amount, "insufficient allowance");
+        require(balanceOf[from] >= amount, "insufficient balance");
+
+        allowance[from][msg.sender] = allowed - amount;
+        balanceOf[from] -= amount;
+        balanceOf[to] += amount;
+        return true;
+    }
+}
+
+/// @title SafeERC20Approval - fixture that supports delta-based allowance changes
+/// @notice Rejects direct non-zero to non-zero approve changes and exposes
+///         increase/decrease allowance helpers for safe adjustment.
+contract SafeERC20Approval is VulnerableERC20Approval {
+    mapping(address => mapping(address => bool)) public hasApprovedSpender;
+
+    function approve(address spender, uint256 amount) public override returns (bool) {
+        uint256 current = allowance[msg.sender][spender];
+        require(!hasApprovedSpender[msg.sender][spender] || amount == 0, "use increase/decrease allowance");
+        require(current == 0 || amount == 0, "use increase/decrease allowance");
+        allowance[msg.sender][spender] = amount;
+        if (amount > 0) {
+            hasApprovedSpender[msg.sender][spender] = true;
+        }
+        return true;
+    }
+
+    function increaseAllowance(address spender, uint256 addedValue) external returns (bool) {
+        hasApprovedSpender[msg.sender][spender] = true;
+        allowance[msg.sender][spender] += addedValue;
+        return true;
+    }
+
+    function decreaseAllowance(address spender, uint256 subtractedValue) external returns (bool) {
+        uint256 current = allowance[msg.sender][spender];
+        require(current >= subtractedValue, "decreased allowance below zero");
+        allowance[msg.sender][spender] = current - subtractedValue;
+        return true;
+    }
+}