From 0e264b3b84b90d08b03c44183331d0cf2bc8503f Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 2 Sep 2022 20:34:42 -0500 Subject: [PATCH] consider state variable collisions and incorrect return type --- slither/core/variables/state_variable.py | 44 ++- .../permit_domain_signature_collision.py | 27 +- ...n.sol.0.4.25.DomainSeparatorCollision.json | 6 +- .../permit_domain_state_var_collision.sol | 205 ++++++++++++++ ...n.sol.0.4.25.DomainSeparatorCollision.json | 247 +++++++++++++++++ .../permit_domain_wrong_return_type.sol | 207 ++++++++++++++ ...e.sol.0.4.25.DomainSeparatorCollision.json | 252 ++++++++++++++++++ ...n.sol.0.5.16.DomainSeparatorCollision.json | 6 +- .../permit_domain_state_var_collision.sol | 205 ++++++++++++++ ...n.sol.0.5.16.DomainSeparatorCollision.json | 247 +++++++++++++++++ .../permit_domain_wrong_return_type.sol | 207 ++++++++++++++ ...e.sol.0.5.16.DomainSeparatorCollision.json | 252 ++++++++++++++++++ ...n.sol.0.6.11.DomainSeparatorCollision.json | 6 +- .../permit_domain_state_var_collision.sol | 205 ++++++++++++++ ...n.sol.0.6.11.DomainSeparatorCollision.json | 247 +++++++++++++++++ .../permit_domain_wrong_return_type.sol | 207 ++++++++++++++ ...e.sol.0.6.11.DomainSeparatorCollision.json | 252 ++++++++++++++++++ ...on.sol.0.7.6.DomainSeparatorCollision.json | 6 +- .../permit_domain_state_var_collision.sol | 205 ++++++++++++++ ...on.sol.0.7.6.DomainSeparatorCollision.json | 247 +++++++++++++++++ .../0.7.6/permit_domain_wrong_return_type.sol | 207 ++++++++++++++ ...pe.sol.0.7.6.DomainSeparatorCollision.json | 252 ++++++++++++++++++ ...on.sol.0.8.0.DomainSeparatorCollision.json | 6 +- .../permit_domain_state_var_collision.sol | 205 ++++++++++++++ ...on.sol.0.8.0.DomainSeparatorCollision.json | 247 +++++++++++++++++ .../0.8.0/permit_domain_wrong_return_type.sol | 207 ++++++++++++++ ...pe.sol.0.8.0.DomainSeparatorCollision.json | 252 ++++++++++++++++++ tests/test_detectors.py | 50 ++++ tests/test_function.py | 18 +- tests/test_function.sol | 1 + 30 files changed, 4696 insertions(+), 29 deletions(-) create mode 100644 tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol create mode 100644 tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol.0.4.25.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol create mode 100644 tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol.0.4.25.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol create mode 100644 tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol.0.5.16.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol create mode 100644 tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol.0.5.16.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol create mode 100644 tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol.0.6.11.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol create mode 100644 tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol.0.6.11.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol create mode 100644 tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol.0.7.6.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol create mode 100644 tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol.0.7.6.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol create mode 100644 tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol.0.8.0.DomainSeparatorCollision.json create mode 100644 tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol create mode 100644 tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol.0.8.0.DomainSeparatorCollision.json diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index c9a90f36b..5fd600be0 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -1,7 +1,9 @@ -from typing import Optional, TYPE_CHECKING +from typing import Optional, TYPE_CHECKING, Tuple, List from slither.core.children.child_contract import ChildContract from slither.core.variables.variable import Variable +from slither.utils.type import export_nested_types_from_variable +from slither.core.solidity_types.type import Type if TYPE_CHECKING: from slither.core.cfg.node import Node @@ -21,6 +23,46 @@ class StateVariable(ChildContract, Variable): """ return self.contract == contract + ################################################################################### + ################################################################################### + # region Signature and return type of state variable getters + ################################################################################### + ################################################################################### + + @property + def signature(self) -> Tuple[str, List[str], List[str]]: + """ + Return the signature of the state variable as a function signature + :return: (str, list(str), list(str)), as (name, list parameters type, list return values type) + """ + return ( + self.name, + [str(x) for x in export_nested_types_from_variable(self)], + [str(self.type)], + ) + + @property + def signature_str(self) -> str: + """ + Return the signature of the state variable as a function signature + :return: str: func_name(type1,type2) returns(type3) + """ + name, parameters, returnVars = self.signature + return name + "(" + ",".join(parameters) + ") returns(" + ",".join(returnVars) + ")" + + @property + def solidity_signature(self) -> Optional[str]: + if self.visibility in ["public", "external"]: + name, parameters, _ = self.signature + return f"{name}({','.join(parameters)})" + return None + + @property + def return_type(self) -> Optional[List[Type]]: + if self.visibility in ["public", "external"]: + return [self.type] + return None + # endregion ################################################################################### ################################################################################### diff --git a/slither/detectors/functions/permit_domain_signature_collision.py b/slither/detectors/functions/permit_domain_signature_collision.py index 9e2301a45..3a4fdd2f9 100644 --- a/slither/detectors/functions/permit_domain_signature_collision.py +++ b/slither/detectors/functions/permit_domain_signature_collision.py @@ -3,6 +3,7 @@ Module detecting EIP-2612 domain separator collision """ from slither.utils.function import get_function_id from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.core.solidity_types.elementary_type import ElementaryType class DomainSeparatorCollision(AbstractDetector): @@ -35,21 +36,29 @@ contract Contract{ WIKI_RECOMMENDATION = "Remove or rename the function that collides with DOMAIN_SEPARATOR()." def _detect(self): - results = [] domain_sig = get_function_id("DOMAIN_SEPARATOR()") for contract in self.compilation_unit.contracts_derived: if contract.is_erc20(): - for func in contract.functions: - if ( + for func in contract.functions_entry_points + contract.state_variables: + # Skip internal and private variables + if func.solidity_signature is None: + continue + # External/ public function names should not collide with DOMAIN_SEPARATOR() + hash_collision = ( func.solidity_signature != "DOMAIN_SEPARATOR()" and get_function_id(func.solidity_signature) == domain_sig - ): + ) + # DOMAIN_SEPARATOR() should return bytes32 + incorrect_return_type = ( + func.solidity_signature == "DOMAIN_SEPARATOR()" + and func.return_type[0] != ElementaryType("bytes32") + ) + if hash_collision or incorrect_return_type: info = [ + "The function signature of ", func, - "'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + " collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", ] res = self.generate_result(info) - results.append(res) - break - - return results + return [res] + return [] diff --git a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol.0.4.25.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol.0.4.25.DomainSeparatorCollision.json index ef58ab72b..7b89af4a8 100644 --- a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol.0.4.25.DomainSeparatorCollision.json +++ b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol.0.4.25.DomainSeparatorCollision.json @@ -240,10 +240,10 @@ } } ], - "description": "ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol#161-163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", - "markdown": "[ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol#L161-L163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "description": "The function signature of ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", "first_markdown_element": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_collision.sol#L161-L163", - "id": "89af79b93e8de48fb8f88ab4dabbc97b0bb35ab39b6544e255b16aa924286eab", + "id": "cb8ae27add92ad3163cbe9c0fb29a2a0032ba46384bbd5541d1d750251f5c83e", "check": "domain-separator-collision", "impact": "Medium", "confidence": "High" diff --git a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol new file mode 100644 index 000000000..0faf421a6 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + bytes32 public fopwCDKKK; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + fopwCDKKK, + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol.0.4.25.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol.0.4.25.DomainSeparatorCollision.json new file mode 100644 index 000000000..5d4c00817 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol.0.4.25.DomainSeparatorCollision.json @@ -0,0 +1,247 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "fopwCDKKK", + "source_mapping": { + "start": 1735, + "length": 24, + "filename_relative": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 46 + ], + "starting_column": 5, + "ending_column": 29 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6054, + "filename_relative": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "The function signature of ERC20.fopwCDKKK (tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol#46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK](tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol#L46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_state_var_collision.sol#L46", + "id": "8d18da367a9cfe0bee2ee48ee8a76072af23567d852cc81ed75dd90531cbe3d5", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol new file mode 100644 index 000000000..5d3401871 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view returns (uint64) { + return uint64(1); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol.0.4.25.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol.0.4.25.DomainSeparatorCollision.json new file mode 100644 index 000000000..f5a460429 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol.0.4.25.DomainSeparatorCollision.json @@ -0,0 +1,252 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "DOMAIN_SEPARATOR", + "source_mapping": { + "start": 5248, + "length": 90, + "filename_relative": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 161, + 162, + 163 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6128, + "filename_relative": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203, + 204, + 205 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "DOMAIN_SEPARATOR()" + } + } + ], + "description": "The function signature of ERC20.DOMAIN_SEPARATOR() (tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.DOMAIN_SEPARATOR()](tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.4.25/permit_domain_wrong_return_type.sol#L161-L163", + "id": "17ee24b60ef7d108871021639c374d6711feb1c8e3aad52ab266a680c03831cb", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol.0.5.16.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol.0.5.16.DomainSeparatorCollision.json index d94ef48b9..23f93fd7e 100644 --- a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol.0.5.16.DomainSeparatorCollision.json +++ b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol.0.5.16.DomainSeparatorCollision.json @@ -240,10 +240,10 @@ } } ], - "description": "ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol#161-163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", - "markdown": "[ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol#L161-L163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "description": "The function signature of ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", "first_markdown_element": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_collision.sol#L161-L163", - "id": "89af79b93e8de48fb8f88ab4dabbc97b0bb35ab39b6544e255b16aa924286eab", + "id": "cb8ae27add92ad3163cbe9c0fb29a2a0032ba46384bbd5541d1d750251f5c83e", "check": "domain-separator-collision", "impact": "Medium", "confidence": "High" diff --git a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol new file mode 100644 index 000000000..2c63161f4 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + bytes32 public fopwCDKKK; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + fopwCDKKK, + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol.0.5.16.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol.0.5.16.DomainSeparatorCollision.json new file mode 100644 index 000000000..5cc2627ae --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol.0.5.16.DomainSeparatorCollision.json @@ -0,0 +1,247 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "fopwCDKKK", + "source_mapping": { + "start": 1735, + "length": 24, + "filename_relative": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 46 + ], + "starting_column": 5, + "ending_column": 29 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6061, + "filename_relative": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "The function signature of ERC20.fopwCDKKK (tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol#46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK](tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol#L46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_state_var_collision.sol#L46", + "id": "8d18da367a9cfe0bee2ee48ee8a76072af23567d852cc81ed75dd90531cbe3d5", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol new file mode 100644 index 000000000..c3f31e5f8 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view returns (uint64) { + return uint64(1); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol.0.5.16.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol.0.5.16.DomainSeparatorCollision.json new file mode 100644 index 000000000..20ab9665d --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol.0.5.16.DomainSeparatorCollision.json @@ -0,0 +1,252 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "DOMAIN_SEPARATOR", + "source_mapping": { + "start": 5255, + "length": 90, + "filename_relative": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 161, + 162, + 163 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6135, + "filename_relative": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203, + 204, + 205 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "DOMAIN_SEPARATOR()" + } + } + ], + "description": "The function signature of ERC20.DOMAIN_SEPARATOR() (tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.DOMAIN_SEPARATOR()](tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.5.16/permit_domain_wrong_return_type.sol#L161-L163", + "id": "17ee24b60ef7d108871021639c374d6711feb1c8e3aad52ab266a680c03831cb", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol.0.6.11.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol.0.6.11.DomainSeparatorCollision.json index 5039fdc19..8bbb29bb9 100644 --- a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol.0.6.11.DomainSeparatorCollision.json +++ b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol.0.6.11.DomainSeparatorCollision.json @@ -240,10 +240,10 @@ } } ], - "description": "ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol#161-163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", - "markdown": "[ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol#L161-L163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "description": "The function signature of ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", "first_markdown_element": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_collision.sol#L161-L163", - "id": "89af79b93e8de48fb8f88ab4dabbc97b0bb35ab39b6544e255b16aa924286eab", + "id": "cb8ae27add92ad3163cbe9c0fb29a2a0032ba46384bbd5541d1d750251f5c83e", "check": "domain-separator-collision", "impact": "Medium", "confidence": "High" diff --git a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol new file mode 100644 index 000000000..2c63161f4 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + bytes32 public fopwCDKKK; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + fopwCDKKK, + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol.0.6.11.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol.0.6.11.DomainSeparatorCollision.json new file mode 100644 index 000000000..ace6f711a --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol.0.6.11.DomainSeparatorCollision.json @@ -0,0 +1,247 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "fopwCDKKK", + "source_mapping": { + "start": 1735, + "length": 24, + "filename_relative": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 46 + ], + "starting_column": 5, + "ending_column": 29 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6061, + "filename_relative": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "The function signature of ERC20.fopwCDKKK (tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol#46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK](tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol#L46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_state_var_collision.sol#L46", + "id": "8d18da367a9cfe0bee2ee48ee8a76072af23567d852cc81ed75dd90531cbe3d5", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol new file mode 100644 index 000000000..c3f31e5f8 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view returns (uint64) { + return uint64(1); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol.0.6.11.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol.0.6.11.DomainSeparatorCollision.json new file mode 100644 index 000000000..7da702ba4 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol.0.6.11.DomainSeparatorCollision.json @@ -0,0 +1,252 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "DOMAIN_SEPARATOR", + "source_mapping": { + "start": 5255, + "length": 90, + "filename_relative": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 161, + 162, + 163 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6135, + "filename_relative": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203, + 204, + 205 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "DOMAIN_SEPARATOR()" + } + } + ], + "description": "The function signature of ERC20.DOMAIN_SEPARATOR() (tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.DOMAIN_SEPARATOR()](tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.6.11/permit_domain_wrong_return_type.sol#L161-L163", + "id": "17ee24b60ef7d108871021639c374d6711feb1c8e3aad52ab266a680c03831cb", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol.0.7.6.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol.0.7.6.DomainSeparatorCollision.json index 9f0c1db41..6f42fc7a6 100644 --- a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol.0.7.6.DomainSeparatorCollision.json +++ b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol.0.7.6.DomainSeparatorCollision.json @@ -240,10 +240,10 @@ } } ], - "description": "ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol#161-163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", - "markdown": "[ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol#L161-L163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "description": "The function signature of ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", "first_markdown_element": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_collision.sol#L161-L163", - "id": "89af79b93e8de48fb8f88ab4dabbc97b0bb35ab39b6544e255b16aa924286eab", + "id": "cb8ae27add92ad3163cbe9c0fb29a2a0032ba46384bbd5541d1d750251f5c83e", "check": "domain-separator-collision", "impact": "Medium", "confidence": "High" diff --git a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol new file mode 100644 index 000000000..2c63161f4 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + bytes32 public fopwCDKKK; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + fopwCDKKK, + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol.0.7.6.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol.0.7.6.DomainSeparatorCollision.json new file mode 100644 index 000000000..9c246aca1 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol.0.7.6.DomainSeparatorCollision.json @@ -0,0 +1,247 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "fopwCDKKK", + "source_mapping": { + "start": 1735, + "length": 24, + "filename_relative": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 46 + ], + "starting_column": 5, + "ending_column": 29 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6061, + "filename_relative": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "The function signature of ERC20.fopwCDKKK (tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol#46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK](tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol#L46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_state_var_collision.sol#L46", + "id": "8d18da367a9cfe0bee2ee48ee8a76072af23567d852cc81ed75dd90531cbe3d5", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol new file mode 100644 index 000000000..5d3401871 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view returns (uint64) { + return uint64(1); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol.0.7.6.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol.0.7.6.DomainSeparatorCollision.json new file mode 100644 index 000000000..957ae1920 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol.0.7.6.DomainSeparatorCollision.json @@ -0,0 +1,252 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "DOMAIN_SEPARATOR", + "source_mapping": { + "start": 5248, + "length": 90, + "filename_relative": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 161, + 162, + 163 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6128, + "filename_relative": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203, + 204, + 205 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "DOMAIN_SEPARATOR()" + } + } + ], + "description": "The function signature of ERC20.DOMAIN_SEPARATOR() (tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.DOMAIN_SEPARATOR()](tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.7.6/permit_domain_wrong_return_type.sol#L161-L163", + "id": "17ee24b60ef7d108871021639c374d6711feb1c8e3aad52ab266a680c03831cb", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol.0.8.0.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol.0.8.0.DomainSeparatorCollision.json index bc816b0d6..5d6f68aa9 100644 --- a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol.0.8.0.DomainSeparatorCollision.json +++ b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol.0.8.0.DomainSeparatorCollision.json @@ -240,10 +240,10 @@ } } ], - "description": "ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol#161-163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", - "markdown": "[ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol#L161-L163)'s function signature collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "description": "The function signature of ERC20.fopwCDKKK() (tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK()](tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", "first_markdown_element": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_collision.sol#L161-L163", - "id": "89af79b93e8de48fb8f88ab4dabbc97b0bb35ab39b6544e255b16aa924286eab", + "id": "cb8ae27add92ad3163cbe9c0fb29a2a0032ba46384bbd5541d1d750251f5c83e", "check": "domain-separator-collision", "impact": "Medium", "confidence": "High" diff --git a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol new file mode 100644 index 000000000..2c63161f4 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + bytes32 public fopwCDKKK; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) public { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + fopwCDKKK, + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol.0.8.0.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol.0.8.0.DomainSeparatorCollision.json new file mode 100644 index 000000000..dc39ccea9 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol.0.8.0.DomainSeparatorCollision.json @@ -0,0 +1,247 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "fopwCDKKK", + "source_mapping": { + "start": 1735, + "length": 24, + "filename_relative": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 46 + ], + "starting_column": 5, + "ending_column": 29 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6061, + "filename_relative": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "The function signature of ERC20.fopwCDKKK (tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol#46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.fopwCDKKK](tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol#L46) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_state_var_collision.sol#L46", + "id": "8d18da367a9cfe0bee2ee48ee8a76072af23567d852cc81ed75dd90531cbe3d5", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol new file mode 100644 index 000000000..5d3401871 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +/// @notice Modern and gas efficient ERC20 + EIP-2612 implementation. +/// @author Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +/// @author Modified from Uniswap (https://github.com/Uniswap/uniswap-v2-core/blob/master/contracts/UniswapV2ERC20.sol) +/// @dev Do not manually set balances without updating totalSupply, as the sum of all user balances must not exceed it. +contract ERC20 { + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + event Transfer(address indexed from, address indexed to, uint256 amount); + + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string public name; + + string public symbol; + + uint8 public decimals; + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 public totalSupply; + + mapping(address => uint256) public balanceOf; + + mapping(address => mapping(address => uint256)) public allowance; + + /*////////////////////////////////////////////////////////////// + EIP-2612 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal INITIAL_CHAIN_ID; + + bytes32 internal INITIAL_DOMAIN_SEPARATOR; + + mapping(address => uint256) public nonces; + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor( + string memory _name, + string memory _symbol, + uint8 _decimals + ) { + name = _name; + symbol = _symbol; + decimals = _decimals; + + INITIAL_CHAIN_ID = 1; + INITIAL_DOMAIN_SEPARATOR = computeDomainSeparator(); + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve(address spender, uint256 amount) public returns (bool) { + allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer(address to, uint256 amount) public returns (bool) { + balanceOf[msg.sender] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public returns (bool) { + uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != 115792089237316195423570985008687907853269984665640564039457584007913129639935) allowance[from][msg.sender] = allowed - amount; + + balanceOf[from] -= amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(from, to, amount); + + return true; + } + + /*////////////////////////////////////////////////////////////// + EIP-2612 LOGIC + //////////////////////////////////////////////////////////////*/ + + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public { + require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED"); + + // Unchecked because the only math done is incrementing + // the owner's nonce which cannot realistically overflow. + + address recoveredAddress = ecrecover( + keccak256( + abi.encodePacked( + "\x19\x01", + DOMAIN_SEPARATOR(), + keccak256( + abi.encode( + keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ), + owner, + spender, + value, + nonces[owner]++, + deadline + ) + ) + ) + ), + v, + r, + s + ); + + require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER"); + + allowance[recoveredAddress][spender] = value; + + + emit Approval(owner, spender, value); + } + + function DOMAIN_SEPARATOR() public view returns (uint64) { + return uint64(1); + } + + function computeDomainSeparator() internal view returns (bytes32) { + return + keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256(bytes(name)), + keccak256("1"), + 1, + address(this) + ) + ); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL MINT/BURN LOGIC + //////////////////////////////////////////////////////////////*/ + + function _mint(address to, uint256 amount) internal { + totalSupply += amount; + + // Cannot overflow because the sum of all user + // balances can't exceed the max uint256 value. + + balanceOf[to] += amount; + + + emit Transfer(address(0), to, amount); + } + + function _burn(address from, uint256 amount) internal { + balanceOf[from] -= amount; + + // Cannot underflow because a user's balance + // will never be larger than the total supply. + + totalSupply -= amount; + + + emit Transfer(from, address(0), amount); + } +} + +contract Test is ERC20("TEST", "TEST", 18) {} diff --git a/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol.0.8.0.DomainSeparatorCollision.json b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol.0.8.0.DomainSeparatorCollision.json new file mode 100644 index 000000000..c224777b6 --- /dev/null +++ b/tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol.0.8.0.DomainSeparatorCollision.json @@ -0,0 +1,252 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "DOMAIN_SEPARATOR", + "source_mapping": { + "start": 5248, + "length": 90, + "filename_relative": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 161, + 162, + 163 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "ERC20", + "source_mapping": { + "start": 449, + "length": 6128, + "filename_relative": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25, + 26, + 27, + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55, + 56, + 57, + 58, + 59, + 60, + 61, + 62, + 63, + 64, + 65, + 66, + 67, + 68, + 69, + 70, + 71, + 72, + 73, + 74, + 75, + 76, + 77, + 78, + 79, + 80, + 81, + 82, + 83, + 84, + 85, + 86, + 87, + 88, + 89, + 90, + 91, + 92, + 93, + 94, + 95, + 96, + 97, + 98, + 99, + 100, + 101, + 102, + 103, + 104, + 105, + 106, + 107, + 108, + 109, + 110, + 111, + 112, + 113, + 114, + 115, + 116, + 117, + 118, + 119, + 120, + 121, + 122, + 123, + 124, + 125, + 126, + 127, + 128, + 129, + 130, + 131, + 132, + 133, + 134, + 135, + 136, + 137, + 138, + 139, + 140, + 141, + 142, + 143, + 144, + 145, + 146, + 147, + 148, + 149, + 150, + 151, + 152, + 153, + 154, + 155, + 156, + 157, + 158, + 159, + 160, + 161, + 162, + 163, + 164, + 165, + 166, + 167, + 168, + 169, + 170, + 171, + 172, + 173, + 174, + 175, + 176, + 177, + 178, + 179, + 180, + 181, + 182, + 183, + 184, + 185, + 186, + 187, + 188, + 189, + 190, + 191, + 192, + 193, + 194, + 195, + 196, + 197, + 198, + 199, + 200, + 201, + 202, + 203, + 204, + 205 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "DOMAIN_SEPARATOR()" + } + } + ], + "description": "The function signature of ERC20.DOMAIN_SEPARATOR() (tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol#161-163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "markdown": "The function signature of [ERC20.DOMAIN_SEPARATOR()](tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol#L161-L163) collides with DOMAIN_SEPARATOR and should be renamed or removed.\n", + "first_markdown_element": "tests/detectors/domain-separator-collision/0.8.0/permit_domain_wrong_return_type.sol#L161-L163", + "id": "17ee24b60ef7d108871021639c374d6711feb1c8e3aad52ab266a680c03831cb", + "check": "domain-separator-collision", + "impact": "Medium", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 440c8a6ad..970c1c1a0 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -1468,6 +1468,56 @@ ALL_TEST_OBJECTS = [ "permit_domain_collision.sol", "0.8.0", ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_wrong_return_type.sol", + "0.4.25", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_wrong_return_type.sol", + "0.5.16", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_wrong_return_type.sol", + "0.6.11", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_wrong_return_type.sol", + "0.7.6", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_wrong_return_type.sol", + "0.8.0", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_state_var_collision.sol", + "0.4.25", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_state_var_collision.sol", + "0.5.16", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_state_var_collision.sol", + "0.6.11", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_state_var_collision.sol", + "0.7.6", + ), + Test( + all_detectors.DomainSeparatorCollision, + "permit_domain_state_var_collision.sol", + "0.8.0", + ), ] diff --git a/tests/test_function.py b/tests/test_function.py index 19fa596ab..632096a97 100644 --- a/tests/test_function.py +++ b/tests/test_function.py @@ -15,10 +15,7 @@ def test_functions(): # pylint: disable=too-many-statements solc_select.switch_global_version("0.6.12", always_install=True) slither = Slither("tests/test_function.sol") - compilation_unit = slither.compilation_units[0] - functions = compilation_unit.get_contract_from_name("TestFunction")[ - 0 - ].available_functions_as_dict() + functions = slither.get_contract_from_name("TestFunction")[0].available_functions_as_dict() f = functions["external_payable(uint256)"] assert f.name == "external_payable" @@ -267,3 +264,16 @@ def test_function_can_send_eth(): assert functions["transfer_via_external()"].can_send_eth() is False assert functions["call_via_external()"].can_send_eth() is False assert functions["highlevel_call_via_external()"].can_send_eth() is False + + +def test_public_variable(): + solc_select.switch_global_version("0.6.12", always_install=True) + slither = Slither("tests/test_function.sol") + contracts = slither.get_contract_from_name("TestFunction") + assert len(contracts) == 1 + contract = contracts[0] + var = contract.get_state_variable_from_name("info") + assert var.solidity_signature == "info()" + assert var.signature_str == "info() returns(bytes32)" + assert var.visibility == "public" + assert var.return_type[0] == ElementaryType("bytes32") diff --git a/tests/test_function.sol b/tests/test_function.sol index 6ea6b9d8a..aca9fc93d 100644 --- a/tests/test_function.sol +++ b/tests/test_function.sol @@ -7,6 +7,7 @@ pragma solidity ^0.6.12; contract TestFunction { bool entered = false; + bytes32 public info; function external_payable(uint _a) external payable returns (uint) { return 1;