diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index de577b08e..34966df42 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -16,6 +16,7 @@ class NamingConvention(AbstractDetector): Exceptions: - Allow constant variables name/symbol/decimals to be lowercase (ERC20) - Allow '_' at the beginning of the mixed_case match for private variables and unused parameters + - Allow '$' (the ERC-7201 namespaced-storage pointer name) for the mixed_case match - Ignore echidna properties (functions with names starting 'echidna_' or 'crytic_' """ @@ -33,7 +34,8 @@ class NamingConvention(AbstractDetector): Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.25/style-guide.html#naming-conventions) that should be followed. #### Rule exceptions - Allow constant variable name/symbol/decimals to be lowercase (`ERC20`). -- Allow `_` at the beginning of the `mixed_case` match for private variables and unused parameters.""" +- Allow `_` at the beginning of the `mixed_case` match for private variables and unused parameters. +- Allow `$` (the [ERC-7201](https://eips.ethereum.org/EIPS/eip-7201) namespaced-storage pointer name) for the `mixed_case` match.""" # endregion wiki_description WIKI_RECOMMENDATION = "Follow the Solidity [naming convention](https://solidity.readthedocs.io/en/v0.4.25/style-guide.html#naming-conventions)." @@ -54,12 +56,21 @@ def is_state_naming(name: str) -> bool: @staticmethod def is_mixed_case(name: str) -> bool: + if name == "$": + # `$` is a valid Solidity identifier and the conventional name for + # the ERC-7201 namespaced-storage struct pointer (used by + # OpenZeppelin, Solady, ...). It has no letters to case, so it + # trivially conforms to mixedCase. + return True return re.search("^[a-z]([A-Za-z0-9]+)?_?$", name) is not None @staticmethod def is_mixed_case_with_underscore(name: str) -> bool: # Allow _ at the beginning to represent private variable # or unused parameters + if name == "$": + # ERC-7201 namespaced-storage pointer, see `is_mixed_case`. + return True return re.search("^[_]?[a-z]([A-Za-z0-9]+)?_?$", name) is not None @staticmethod diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt new file mode 100644 index 000000000..a7c99851c --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt @@ -0,0 +1,2 @@ +Parameter LibExample.tripwire(uint256).bad_param (tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol#33) is not in mixedCase + diff --git a/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol new file mode 100644 index 000000000..37e313170 --- /dev/null +++ b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +/// Regression coverage for the `naming-convention` fix on the ERC-7201 +/// namespaced-storage pointer name `$`. +/// +/// `$` is a valid Solidity identifier and the de-facto convention for the +/// storage-struct pointer (OpenZeppelin upgradeable, Solady, ...). It carries +/// no letters to case, so the mixedCase checks must not flag it. Before the +/// fix, every `function f(SomeStorage storage $)` reported a `Parameter ... .$ +/// is not in mixedCase` false positive (24+ on Solady alone). +/// +/// Expected output after fix: exactly one finding, for the deliberately +/// snake_cased parameter `bad_param` (the tripwire proving the detector still +/// flags genuine Solidity-level violations). + +library LibExample { + struct BytesStorage { + bytes32 _spacer; + } + + // `$` is read here -> exercises `is_mixed_case` on the storage pointer. + function length(BytesStorage storage $) internal view returns (uint256 result) { + assembly { + result := shr(224, sload($.slot)) + } + } + + // `$` is unused here -> exercises `is_mixed_case_with_underscore`. + function clear(BytesStorage storage $) internal {} + + // Tripwire: a snake_cased parameter must still be reported. + function tripwire(uint256 bad_param) internal pure returns (uint256) { + return bad_param; + } +} diff --git a/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip new file mode 100644 index 000000000..0c989d907 Binary files /dev/null and b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index 7f0756ff8..5129f12fd 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -644,6 +644,11 @@ def id_test(test_item: Test): "naming_convention_yul_local.sol", "0.8.20", ), + Test( + all_detectors.NamingConvention, + "naming_convention_dollar_storage.sol", + "0.8.20", + ), Test( all_detectors.ControlledDelegateCall, "controlled_delegatecall.sol",