Merge pull request #5 from forefy/feature

Templates Contribution by @scab24

Author: forefy-old

eburger/templates/emit_after_external_call.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Emit After External Call"
 severity: "Low"
 precision: "Medium"

eburger/templates/missing_reentrancy_guard.yaml

@@ -1,4 +1,5 @@
 version: 1.0.6
+author: "@forefy"
 name: "Missing Reentracy Guard"
 severity: "Low"
 precision: "Low"

eburger/templates/missing_zero_address_check.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Missing Zero Address Check"
 severity: "Low"
 precision: "Medium"

eburger/templates/modifier_without_proper_enforcement.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Modifier Without Proper Enforcement"
 severity: "High"
 precision: "Low"

eburger/templates/tx_origin_used_for_access_control.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "tx.origin Used for Access Control"
 severity: "Low"
 precision: "Medium"

eburger/templates/unbounded_loop.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Unbounded Loop"
 severity: "Low"
 precision: "Low"

eburger/templates/unchecked_call_return.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Unchecked Call Return"
 severity: "Low"
 precision: "Low"

eburger/templates/unchecked_chainlink_oracle_price.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Unchecked Chainlink Oracle Price"
 severity: "Medium"
 precision: "Medium"

eburger/templates/unspecific_pragma_detector.yaml

@@ -0,0 +1,24 @@
+version: 1.0.6
+author: "@Seecoalba"
+name: "Unspecific Solidity Pragma Detector"
+severity: "Low"
+precision: "High"
+description: "Detects the usage of unspecific compiler pragmas that allow for a broader range of compiler versions than necessary, which can lead to unintended behavior or compiler warnings/errors with newer versions."
+impact: "Unpredictable behavior due to differences in compiler versions."
+action-items:
+    - "Specify a more precise range of compiler versions in the pragma statement to ensure consistent behavior across different environments."
+references:
+    - "https://docs.soliditylang.org/en/latest/layout-of-source-files.html#version-pragma"
+reports: 
+    - "https://github.com/Steemhunt/mint.club-v2-contract/issues/36"
+    - "https://solodit.xyz/issues/n-06-inconsistent-method-of-specifying-a-floating-pragma-code4rena-ens-ens-contest-git"
+vulnerable_contracts: 
+    - "../vulnerable_contracts/unspecific_pragma_detector.sol"
+python: |
+    results = []
+    pragma_directives = get_nodes_by_types(ast_data, "PragmaDirective")
+    for directive in pragma_directives:
+        literals = directive.get('literals', [])
+        pragma_string = ''.join(literals)
+        if '^' in pragma_string or '>' in pragma_string:
+            results.append(directive)

eburger/templates/unverified_from_address_in_transfer.yaml

@@ -0,0 +1,54 @@
+version: 1.0.6
+author: "@Seecoalba"
+name: "Unverified from Address Usage in transferFrom"
+severity: "Low"
+precision: "Medium"
+description: "Employing an unverified from address in transferFrom or safeTransferFrom operations poses a substantial risk of fund loss. This risk arises from the potential for any party to execute token transfers from the specified from address, contingent upon having received adequate approval."
+impact: "Unauthorized token transactions emanating from the specified from address, if it has been authorized, which could lead to significant asset depletion."
+action-items:
+    - "Verify that the from address used in calls to transferFrom or safeTransferFrom is validated properly."
+references:
+    - "https://solidity.readthedocs.io/en/latest/types.html#members-of-addresses"
+reports: []
+vulnerable_contracts: 
+    - "../vulnerable_contracts/unverified_from_address_in_transfer/"
+python: |
+    results = []
+    # Retrieve all function call nodes from the AST.
+    function_calls = get_nodes_by_types(ast_data, ["FunctionCall"])
+    
+    # Define a function to check the validity of the 'from' argument in a function call.
+    def check_argument_validity(function_call):
+        # Extract arguments from the function call.
+        args = function_call.get("arguments", [])
+        # Determine the index of the 'from' argument based on the number of arguments.
+        arg_index = 0 if len(args) == 3 else 1 if len(args) == 4 else -1
+        
+        # Ensure the arg_index is valid.
+        if arg_index not in [-1, len(args)]:
+            # Get the 'from' argument based on its index.
+            arg = args[arg_index]
+            # If the 'from' argument is accessed via a member access,
+            # verify if it securely originates from msg.sender.
+            if arg.get("nodeType") == "MemberAccess":
+                base_expr = arg.get("expression", {})
+                if arg.get("memberName") == "sender" and base_expr.get("name") == "msg":
+                    return False
+            # If the 'from' argument is a result of a function call,
+            # verify if it safely refers to 'this'.
+            elif arg.get("nodeType") == "FunctionCall":
+                expr = arg.get("expression", {})
+                if expr.get("typeName", {}).get("name") == "address" and \
+                   arg.get("arguments", [])[0].get("name") == "this":
+                    return False
+        # Consider the call valid if none of the above conditions are met.
+        return True
+    
+    # Iterate through each function call found in the AST.
+    for function_call in function_calls:
+        # Extract the name of the function being called.
+        function_name = function_call.get("expression", {}).get("memberName", "")
+        # If the function is 'transferFrom' or 'safeTransferFrom', and the 'from' argument is considered valid,
+        # add the function call to the results.
+        if function_name in ["transferFrom", "safeTransferFrom"] and check_argument_validity(function_call):
+            results.append(function_call)

eburger/templates/use_of_approve_with_max_allowance.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Use of approve with Max Allowance"
 severity: "Low"
 precision: "High"

eburger/templates/use_of_encodepacked.yaml

@@ -0,0 +1,41 @@
+version: 1.0.6
+author: "@Seecoalba"
+name: "Use of encodedPacked with Dynamic Data Types"
+severity: "Low"
+precision: "Medium"
+description: "It's recommended to avoid `abi.encodePacked()` for dynamic data types before hashing operations, like with `keccak256()`."
+impact: "Potential triggering of hash collisions."
+action-items:
+    - "Unless encodePacked is necessary, switch from `abi.encodePacked()` to `abi.encode()` in scenarios involving dynamic types ahead of hash function usage."
+    - "Consider `bytes.concat()` for concatenating strings or bytes, as a preferable method over `abi.encodePacked()`."
+references:
+    - "https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode"
+    - "https://ethereum.stackexchange.com/questions/30912/how-to-compare-strings-in-solidity#answer-82739"
+reports: 
+    - "https://github.com/code-423n4/2023-11-shellprotocol/blob/8c6c6a33c817b588567a5b6a65bdda72d905941f/4naly3er-report.md?plain=1#L330"
+    - "https://solodit.xyz/issues/l-07-code4rena-backd-backd-contest-git"
+vulnerable_contracts: 
+    "../vulnerable_contracts/use_of_encodepacked.sol"
+python: |
+    results = []
+    nodes = get_nodes_by_types(ast_data, ["MemberAccess"])
+    for node in nodes:
+        if node.get("memberName") == "encodePacked":
+            uses_dynamic_data_types = False
+            problematic_type_string_patterns = ["bytes memory", "[] memory", "string memory"]
+            argument_types = node.get("argumentTypes", [])
+            for argument_type in argument_types:
+                type_string = argument_type.get("typeString", None)
+
+                pattern_match = False
+                for pattern in problematic_type_string_patterns:
+                    if pattern in type_string:
+                        pattern_match = True
+                        break
+
+                if pattern_match:
+                    uses_dynamic_data_types = True
+                    break
+                   
+            if uses_dynamic_data_types:
+                results.append(node)

eburger/templates/use_of_safetransferlib.yaml

@@ -0,0 +1,24 @@
+version: 1.0.6
+author: "@Seecoalba"
+name: "Use of SafeTransferLib"
+severity: "Low"
+precision: "High"
+description: "A notable distinction exists between Solmate's SafeTransferLib and OpenZeppelin's SafeERC20 library: the latter ensures the target is indeed a contract, a step omitted by Solmate's library. It is crucial to note from the documentation that the functions within this library do not verify the existence of code at the token's address, leaving it up to the user to ensure validity."
+impact: "Omitting checks for the token contract's presence could lead to tokens being sent to addresses that cannot interact with them properly, potentially resulting in the loss of assets or unsuccessful transactions."
+action-items:
+  - "Consider leveraging OpenZeppelin's SafeERC20 for ERC20 token interactions to incorporate contract presence verification."
+references:
+  - "https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol#L9"
+reports: 
+  - "https://github.com/code-423n4/2023-01-astaria-findings/blob/8059d5e7c4b3b7155259c7990226c91dc2f375fb/data/btk-Q.md?plain=1#L11"
+  - "https://solodit.xyz/issues/l-04-solmates-code4rena-caviar-caviar-contest-git"
+  - "https://solodit.xyz/issues/l-03-solmates-safetransferlib-doesnt-check-whether-the-erc20-contract-exists-code4rena-redacted-cartel-redacted-cartel-contest-git"
+vulnerable_contracts: 
+  - "../vulnerable_contracts/use_of_safetransferlib/"
+python: |
+  results = []
+  import_directives = get_nodes_by_types(ast_data, "ImportDirective")
+  for directive in import_directives:
+      absolute_path = directive.get("absolutePath", "")
+      if "solmate" in absolute_path and "SafeTransferLib" in absolute_path:
+          results.append(directive)

eburger/templates/use_of_transfer_or_send_on_payable.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Use of transfer or send on a payable address"
 severity: "Medium"
 precision: "Medium"

eburger/templates/use_of_unsafe_mint.yaml

@@ -1,4 +1,5 @@
 version: 1.0.5
+author: "@forefy"
 name: "Usage of unsafe _mint"
 severity: "Medium"
 precision: "High"

vulnerable_contracts/missing_reentrancy_guard/src/missing_reentrancy_guard.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.20;
+pragma solidity ^0.8.20;
 
 contract ProblematicContract {
     mapping(address => uint) public balances;

vulnerable_contracts/unspecific_pragma_detector.sol

@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+contract OutOfControlContract {}

vulnerable_contracts/unverified_from_address_in_transfer/README.md

@@ -0,0 +1,28 @@
+# Unverified from Address Usage in transferFrom
+An intentionally vulnerable Foundry project.
+
+```bash
+tree
+.
+├── foundry.toml                                       # Contract's root
+├── src/                                    
+│   └── unverified_from_address_in_transfer.sol  # Vulnerable contract
+```
+## Usage
+
+### Install dependencies
+```bash
+forge install OpenZeppelin/openzeppelin-contracts --no-git
+```
+
+### Build
+
+```bash
+forge build
+```
+
+### Test
+
+```bash
+forge test
+```

vulnerable_contracts/unverified_from_address_in_transfer/foundry.toml

@@ -0,0 +1,6 @@
+[profile.default]
+src = "src"
+out = "out"
+libs = ["lib"]
+
+# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

vulnerable_contracts/unverified_from_address_in_transfer/src/unverified_from_address_in_transfer.sol

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.20;
+
+import "../lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
+import "../lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
+
+contract InsecureTokenTransfer {
+    using SafeERC20 for IERC20;
+
+    IERC20 private _token;
+
+    constructor(IERC20 tokenAddress) {
+        _token = tokenAddress;
+    }
+
+    function insecureTransfer(address sender, address recipient, uint256 value) public {
+        _token.transferFrom(sender, recipient, value);
+    }
+
+    // Methods below are intended to show secure use cases
+    function secureTransferTo(address recipient, uint256 value) public {
+        // transferring from the function caller can give more control over the from address
+        _token.transferFrom(msg.sender, recipient, value);
+    }
+
+}

vulnerable_contracts/use_of_encodepacked.sol

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.20;
+
+contract BadCode {
+    function hashCollision1(string memory input1, string memory input2) public pure returns (bytes32) {
+        return keccak256(abi.encodePacked(input1, input2));
+    }
+
+    function hashCollision2(bytes memory input1, uint256[] memory input2) public pure returns (bytes32) {
+        return keccak256(abi.encodePacked(input1, input2));
+    }
+
+    function noHashCollision(string memory input1, string memory input2) public pure returns (bytes32) {
+        return keccak256(abi.encode(input1, input2));
+    }
+}

vulnerable_contracts/use_of_safetransferlib/README.md

@@ -0,0 +1,28 @@
+# Use of SafeTransferLib
+An intentionally vulnerable Foundry project.
+
+```bash
+tree
+.
+├── foundry.toml                                       # Contract's root
+├── src/                                    
+│   └── use_of_safetransferlib.sol  # Vulnerable contract
+```
+## Usage
+
+### Install dependencies
+```bash
+forge install transmissions11/solmate --no-git
+```
+
+### Build
+
+```bash
+forge build
+```
+
+### Test
+
+```bash
+forge test
+```

vulnerable_contracts/use_of_safetransferlib/foundry.toml

@@ -0,0 +1,6 @@
+[profile.default]
+src = "src"
+out = "out"
+libs = ["lib"]
+
+# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

vulnerable_contracts/use_of_safetransferlib/src/use_of_safetransferlib.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.20;
+import {ERC20, SafeTransferLib} from "../lib/solmate/src/utils/SafeTransferLib.sol";
+
+contract NotSoSafe {
+    using SafeTransferLib for ERC20;
+
+    function sendSomeTokens(ERC20 token, address to, uint256 amount) external {
+        token.safeTransfer(to, amount);
+    }
+}