bad transferFrom access control rule

solidity/security/bad-transferFrom-access-control.sol

@@ -0,0 +1,172 @@
+contract Test {
+
+    function func1(address from, address to) public {
+        // ruleid: bad-transferFrom-access-control
+        usdc.transferFrom(from, to, amount);
+    }
+
+    function func2(address from, address to) public {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(owner, pool, amount);
+    }
+
+    function func3(address from, address to) public {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(pool, to, amount);
+    }
+
+    function func4(address from, uint256 amount, address random) public {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(pool, owner, amount);
+    }
+
+    function func5(address from, address to) external {
+        // ruleid: bad-transferFrom-access-control
+        usdc.transferFrom(from, to, amount);
+    }
+
+    function func6(address from, address to) external {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(owner, pool, amount);
+    }
+
+    function func7(address from, address to) external {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(pool, to, amount);
+    }
+
+    function func8(address from, uint256 amount, address random) external {
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(pool, owner, amount);
+    }
+
+    function transferFee(uint256 amount, uint256 feeBps, address token, address from, address to)
+        public
+        returns (uint256)
+    {
+        uint256 fee = calculateFee(amount, feeBps);
+        if (fee > 0) {
+            if (token != NATIVE_TOKEN) {
+                // ERC20 token
+                if (from == address(this)) {
+                    TransferHelper.safeTransfer(token, to, fee);
+                } else {
+                    // safeTransferFrom requires approval
+                    // ruleid: bad-transferFrom-access-control
+                    TransferHelper.transferFrom(token, from, to, fee);
+                }
+            } else {
+                require(from == address(this), "can only transfer eth from the router address");
+
+                // Native ether
+                (bool success,) = to.call{value: fee}("");
+                require(success, "transfer failed in transferFee");
+            }
+            return fee;
+        } else {
+            return 0;
+        }
+    }
+
+    function func9(address from, address to) external {
+        _func10(from, to, amount);
+    }
+
+    function _func10(address from, address to) internal {
+        // ruleid: bad-transferFrom-access-control
+        usdc.transferFrom(from, to, amount);
+    }
+
+
+    // SAFE TRANSFER TESTS
+
+    function func11(address from, address to) public {
+        // ruleid: bad-transferFrom-access-control
+        usdc.safeTransferFrom(from, to, amount);
+    }
+
+    function func12(address from, address to) public {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(owner, pool, amount);
+    }
+
+    function func13(address from, address to) public {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(pool, to, amount);
+    }
+
+    function func14(address from, uint256 amount, address random) public {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(pool, owner, amount);
+    }
+
+    function func15(address from, address to) external {
+        // ruleid: bad-transferFrom-access-control
+        usdc.safeTransferFrom(from, to, amount);
+    }
+
+    function func16(address from, address to) external {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(owner, pool, amount);
+    }
+
+    function func17(address from, address to) external {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(pool, to, amount);
+    }
+
+    function func18(address from, uint256 amount, address random) external {
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(pool, owner, amount);
+    }
+
+    function transferFee2(uint256 amount, uint256 feeBps, address token, address from, address to)
+        public
+        returns (uint256)
+    {
+        uint256 fee = calculateFee(amount, feeBps);
+        if (fee > 0) {
+            if (token != NATIVE_TOKEN) {
+                // ERC20 token
+                if (from == address(this)) {
+                    TransferHelper.safeTransfer(token, to, fee);
+                } else {
+                    // safeTransferFrom requires approval
+                    // ruleid: bad-transferFrom-access-control
+                    TransferHelper.safeTransferFrom(token, from, to, fee);
+                }
+            } else {
+                require(from == address(this), "can only transfer eth from the router address");
+
+                // Native ether
+                (bool success,) = to.call{value: fee}("");
+                require(success, "transfer failed in transferFee");
+            }
+            return fee;
+        } else {
+            return 0;
+        }
+    }
+
+    function func19(address from, address to) external {
+        _func20(from, to, amount);
+    }
+
+    function _func20(address from, address to) internal {
+        // ruleid: bad-transferFrom-access-control
+        usdc.safeTransferFrom(from, to, amount);
+    }
+
+    function _func21(address from, address to) internal {
+        // internal never called
+        // ok: bad-transferFrom-access-control
+        usdc.safeTransferFrom(from, to, amount);
+        // ok: bad-transferFrom-access-control
+        usdc.transferFrom(from, to, amount);
+        // ok: bad-transferFrom-access-control
+        TransferHelper.safeTransferFrom(usdc, from, to, amount);
+        // ok: bad-transferFrom-access-control
+        TransferHelper.transferFrom(usdc, from, to, amount);
+    }
+
+}

solidity/security/bad-transferFrom-access-control.yaml

@@ -0,0 +1,51 @@
+rules:
+  - id: bad-transferFrom-access-control
+    languages:
+      - solidity
+    severity: ERROR
+    message: An attacker may perform transferFrom() with arbitrary addresses
+    metadata:
+      category: security
+      technology:
+        - solidity
+      cwe: "CWE-284: Improper Access Control"
+      confidence: LOW
+      likelihood: HIGH
+      impact: HIGH
+      subcategory:
+        - vuln
+      references: 
+        - https://app.blocksec.com/explorer/tx/eth/0x54f659773dae6e01f83184d4b6d717c7f1bb71c0aa59e8c8f4a57c25271424b3 # YODL hack
+    mode: taint
+    pattern-sources:
+      - label: INPUT_TO
+        pattern-either:
+          - patterns:
+              - pattern: function $F(..., address $FROM, ..., address $TO, ...) public { ... }
+              - focus-metavariable: $TO
+          - patterns:
+              - pattern: function $F(..., address $FROM, ..., address $TO, ...) external { ... }
+              - focus-metavariable: $TO
+      - label: INPUT_FROM
+        pattern-either:
+          - patterns:
+              - pattern: function $F(..., address $FROM, ..., address $TO, ...) public { ... }
+              - focus-metavariable: $FROM
+          - patterns:
+              - pattern: function $F(..., address $FROM, ..., address $TO, ...) external { ... }
+              - focus-metavariable: $FROM
+    pattern-sinks:
+      - requires: INPUT_TO and INPUT_FROM
+        pattern-either:
+          - patterns:
+              - pattern: $TOKEN.transferFrom(...,$FROM,...);
+              - pattern: $TOKEN.transferFrom(...,$TO,...);
+          - patterns:
+              - pattern: $TOKEN.safeTransferFrom(...,$FROM,...);
+              - pattern: $TOKEN.safeTransferFrom(...,$TO,...);
+          - patterns:
+              - pattern: $HELPER.transferFrom($TOKEN,...,$FROM,...);
+              - pattern: $HELPER.transferFrom($TOKEN,...,$TO,...);
+          - patterns:
+              - pattern: $HELPER.safeTransferFrom($TOKEN,...,$FROM,...);
+              - pattern: $HELPER.safeTransferFrom($TOKEN,...,$TO,...);