diff --git a/scripts/ci_test_upgradability.sh b/scripts/ci_test_upgradability.sh index b34564003..0a0d77f51 100755 --- a/scripts/ci_test_upgradability.sh +++ b/scripts/ci_test_upgradability.sh @@ -155,6 +155,32 @@ then exit 255 fi +slither-check-upgradeability "$DIR_TESTS/contractV1_struct.sol" ContractV1 --new-contract-filename "$DIR_TESTS/contractV2_struct.sol" --new-contract-name ContractV2 > test_12.txt 2>&1 +DIFF=$(diff test_12.txt "$DIR_TESTS/test_12.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradeability 12 failed" + cat test_12.txt + echo "" + cat "$DIR_TESTS/test_12.txt" + echo "" + echo "$DIFF" + exit 255 +fi + +slither-check-upgradeability "$DIR_TESTS/contractV1_struct.sol" ContractV1 --new-contract-filename "$DIR_TESTS/contractV2_struct_bug.sol" --new-contract-name ContractV2 > test_13.txt 2>&1 +DIFF=$(diff test_13.txt "$DIR_TESTS/test_13.txt") +if [ "$DIFF" != "" ] +then + echo "slither-check-upgradeability 13 failed" + cat test_13.txt + echo "" + cat "$DIR_TESTS/test_13.txt" + echo "" + echo "$DIFF" + exit 255 +fi + rm test_1.txt rm test_2.txt rm test_3.txt @@ -166,3 +192,5 @@ rm test_8.txt rm test_9.txt rm test_10.txt rm test_11.txt +rm test_12.txt +rm test_13.txt diff --git a/slither/core/declarations/structure.py b/slither/core/declarations/structure.py index 8f6d8c50a..5b514e3cb 100644 --- a/slither/core/declarations/structure.py +++ b/slither/core/declarations/structure.py @@ -51,3 +51,17 @@ class Structure(SourceMapping): def __str__(self) -> str: return self.name + + def __eq__(self, other) -> bool: + if not isinstance(other, Structure): + return False + if len(self.elems) != len(other.elems): + return False + for idx, elem in enumerate(self.elems_ordered): + other_elem = other.elems_ordered[idx] + if str(other_elem.type) != str(elem.type) or other_elem.name != elem.name: + return False + return self.name == other.name + + def __hash__(self): + return hash(self.name) diff --git a/slither/core/solidity_types/user_defined_type.py b/slither/core/solidity_types/user_defined_type.py index 8de41116f..a9bbd40a2 100644 --- a/slither/core/solidity_types/user_defined_type.py +++ b/slither/core/solidity_types/user_defined_type.py @@ -72,8 +72,12 @@ class UserDefinedType(Type): return str(type_used.name) def __eq__(self, other: Any) -> bool: + from slither.core.declarations.contract import Contract + if not isinstance(other, UserDefinedType): return False + if isinstance(self.type, Contract) and isinstance(other.type, Contract): + return self.type == other.type.name return self.type == other.type def __hash__(self) -> int: diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 0a35d8e9f..5e870d92e 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -65,7 +65,6 @@ from slither.visitors.expression.expression import ExpressionVisitor if TYPE_CHECKING: from slither.core.cfg.node import Node - from slither.slithir.operations import Operation logger = logging.getLogger("VISTIOR:ExpressionToSlithIR") diff --git a/tests/check-upgradeability/contractV1_struct.sol b/tests/check-upgradeability/contractV1_struct.sol new file mode 100644 index 000000000..cd0e90bd4 --- /dev/null +++ b/tests/check-upgradeability/contractV1_struct.sol @@ -0,0 +1,8 @@ +contract ContractV1{ + struct Foo { + uint256 bar; + address baz; + } + address destination; + Foo foo; +} diff --git a/tests/check-upgradeability/contractV2_struct.sol b/tests/check-upgradeability/contractV2_struct.sol new file mode 100644 index 000000000..76175d004 --- /dev/null +++ b/tests/check-upgradeability/contractV2_struct.sol @@ -0,0 +1,8 @@ +contract ContractV2{ + struct Foo { + uint256 bar; + address baz; + } + address destination; + Foo foo; +} diff --git a/tests/check-upgradeability/contractV2_struct_bug.sol b/tests/check-upgradeability/contractV2_struct_bug.sol new file mode 100644 index 000000000..2e3b0da34 --- /dev/null +++ b/tests/check-upgradeability/contractV2_struct_bug.sol @@ -0,0 +1,8 @@ +contract ContractV2{ + struct Foo { + uint8 bar; + address baz; + } + address destination; + Foo foo; +} diff --git a/tests/check-upgradeability/test_12.txt b/tests/check-upgradeability/test_12.txt new file mode 100644 index 000000000..353d8ebdb --- /dev/null +++ b/tests/check-upgradeability/test_12.txt @@ -0,0 +1,7 @@ +INFO:Slither: +Initializable contract not found, the contract does not follow a standard initalization schema. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither: +Initializable contract not found, the contract does not follow a standard initalization schema. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither:2 findings, 21 detectors run diff --git a/tests/check-upgradeability/test_13.txt b/tests/check-upgradeability/test_13.txt new file mode 100644 index 000000000..9635f9a43 --- /dev/null +++ b/tests/check-upgradeability/test_13.txt @@ -0,0 +1,12 @@ +INFO:Slither: +Initializable contract not found, the contract does not follow a standard initalization schema. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither: +Different variables between ContractV1 (tests/check-upgradeability/contractV1_struct.sol#1-8) and ContractV2 (tests/check-upgradeability/contractV2_struct_bug.sol#1-8) + ContractV1.foo (tests/check-upgradeability/contractV1_struct.sol#7) + ContractV2.foo (tests/check-upgradeability/contractV2_struct_bug.sol#7) +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#incorrect-variables-with-the-v2 +INFO:Slither: +Initializable contract not found, the contract does not follow a standard initalization schema. +Reference: https://github.com/crytic/slither/wiki/Upgradeability-Checks#initializable-is-missing +INFO:Slither:3 findings, 21 detectors run