From 2bd6cbeddde8dd89a10ecbd3ba3a0f96f994b602 Mon Sep 17 00:00:00 2001 From: Alexis Date: Mon, 8 Apr 2024 10:50:15 +0200 Subject: [PATCH 1/8] Remove deprecated flags and their migration. --- slither/utils/command_line.py | 35 ------------------- .../test_path_filtering/slither.config.json | 2 +- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index f03ced834..a37e85913 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -75,13 +75,6 @@ defaults_flag_in_config = { **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE, } -deprecated_flags = { - "fail_pedantic": True, - "fail_low": False, - "fail_medium": False, - "fail_high": False, -} - def read_config_file(args: argparse.Namespace) -> None: # No config file was provided as an argument @@ -98,12 +91,6 @@ def read_config_file(args: argparse.Namespace) -> None: with open(args.config_file, encoding="utf8") as f: config = json.load(f) for key, elem in config.items(): - if key in deprecated_flags: - logger.info( - yellow(f"{args.config_file} has a deprecated key: {key} : {elem}") - ) - migrate_config_options(args, key, elem) - continue if key not in defaults_flag_in_config: logger.info( yellow(f"{args.config_file} has an unknown key: {key} : {elem}") @@ -118,28 +105,6 @@ def read_config_file(args: argparse.Namespace) -> None: logger.error(yellow("Falling back to the default settings...")) -def migrate_config_options(args: argparse.Namespace, key: str, elem): - if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]: - if key == "fail_pedantic": - pedantic_setting = elem - fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE - setattr(args, "fail_on", fail_on) - logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}") - elif key == "fail_low" and elem is True: - logger.info("Migrating fail_low: true -> fail_on: low") - setattr(args, "fail_on", FailOnLevel.LOW) - - elif key == "fail_medium" and elem is True: - logger.info("Migrating fail_medium: true -> fail_on: medium") - setattr(args, "fail_on", FailOnLevel.MEDIUM) - - elif key == "fail_high" and elem is True: - logger.info("Migrating fail_high: true -> fail_on: high") - setattr(args, "fail_on", FailOnLevel.HIGH) - else: - logger.warning(yellow(f"Key {key} was deprecated but no migration was provided")) - - def output_to_markdown( detector_classes: List[Type[AbstractDetector]], printer_classes: List[Type[AbstractPrinter]], diff --git a/tests/e2e/config/test_path_filtering/slither.config.json b/tests/e2e/config/test_path_filtering/slither.config.json index 95aac92b0..0c365eac7 100644 --- a/tests/e2e/config/test_path_filtering/slither.config.json +++ b/tests/e2e/config/test_path_filtering/slither.config.json @@ -2,6 +2,6 @@ "detectors_to_run": "all", "exclude_informational": true, "exclude_low": true, - "fail_pedantic": false, + "fail_on": "none", "filter_paths": "libs|src/ReentrancyMock.sol" } From dcec99b1b80bb033ccfef2ee17652518d9a5e349 Mon Sep 17 00:00:00 2001 From: Alexis Date: Wed, 10 Apr 2024 11:07:00 +0200 Subject: [PATCH 2/8] Reduce verbosity for InvalidCompilation errors --- slither/__main__.py | 10 ++++++++-- tests/e2e/test_cli.py | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/test_cli.py diff --git a/slither/__main__.py b/slither/__main__.py index caaef5730..4fc83a11e 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -14,7 +14,7 @@ from importlib import metadata from typing import Tuple, Optional, List, Dict, Type, Union, Any, Sequence -from crytic_compile import cryticparser, CryticCompile +from crytic_compile import cryticparser, CryticCompile, InvalidCompilation from crytic_compile.platform.standard import generate_standard_export from crytic_compile.platform.etherscan import SUPPORTED_NETWORK from crytic_compile import compile_all, is_supported @@ -93,7 +93,13 @@ def process_all( detector_classes: List[Type[AbstractDetector]], printer_classes: List[Type[AbstractPrinter]], ) -> Tuple[List[Slither], List[Dict], List[Output], int]: - compilations = compile_all(target, **vars(args)) + + try: + compilations = compile_all(target, **vars(args)) + except InvalidCompilation: + logger.error("Unable to compile all targets.") + sys.exit(2) + slither_instances = [] results_detectors = [] results_printers = [] diff --git a/tests/e2e/test_cli.py b/tests/e2e/test_cli.py new file mode 100644 index 000000000..72e0441d6 --- /dev/null +++ b/tests/e2e/test_cli.py @@ -0,0 +1,20 @@ +import sys +import tempfile +import pytest + +from slither.__main__ import main_impl + + +def test_cli_exit_on_invalid_compilation_file(caplog): + + with tempfile.NamedTemporaryFile("w") as f: + f.write("pragma solidity ^0.10000.0;") + + sys.argv = ["slither", f.name] + with pytest.raises(SystemExit) as pytest_wrapped_e: + main_impl([], []) + + assert pytest_wrapped_e.type == SystemExit + assert pytest_wrapped_e.value.code == 2 + + assert caplog.record_tuples[0] == ("Slither", 40, "Unable to compile all targets.") From 0ecf6b4a1691909b19f46f384d80ea63e8904d91 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 10 Apr 2024 23:12:32 -0500 Subject: [PATCH 3/8] fix: unused state var detector for abstract/library --- .../variables/unused_state_variables.py | 14 +++++++-- ...sedStateVars_0_7_6_unused_state_sol__0.txt | 4 +++ .../unused-state/0.7.6/unused_state.sol | 29 ++++++++++++++++++ .../0.7.6/unused_state.sol-0.7.6.zip | Bin 1874 -> 3063 bytes 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/slither/detectors/variables/unused_state_variables.py b/slither/detectors/variables/unused_state_variables.py index 830ca34ca..0fb068fda 100644 --- a/slither/detectors/variables/unused_state_variables.py +++ b/slither/detectors/variables/unused_state_variables.py @@ -46,8 +46,18 @@ def detect_unused(contract: Contract) -> Optional[List[StateVariable]]: variables_used = [item for sublist in variables_used for item in sublist] variables_used = list(set(variables_used + array_candidates)) + # If the contract is abstract, only consider private variables as other visibilities may be used in dependencies + if contract.is_abstract: + return [ + x + for x in contract.state_variables + if x not in variables_used and x.visibility == "private" + ] + # Return the variables unused that are not public - return [x for x in contract.variables if x not in variables_used and x.visibility != "public"] + return [ + x for x in contract.state_variables if x not in variables_used and x.visibility != "public" + ] class UnusedStateVars(AbstractDetector): @@ -71,7 +81,7 @@ class UnusedStateVars(AbstractDetector): """Detect unused state variables""" results = [] for c in self.compilation_unit.contracts_derived: - if c.is_signature_only(): + if c.is_signature_only() or c.is_library: continue unusedVars = detect_unused(c) if unusedVars: diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt index 39c7ed13e..6e8952883 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt @@ -4,5 +4,9 @@ A.unused4 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#7) A.unused2 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#5) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16) +H.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#38) is never used in I (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#41-46) + +F.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#27) is never used in F (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#26-33) + A.unused3 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#6) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16) diff --git a/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol b/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol index b603f8875..9e4f7ec6f 100644 --- a/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol +++ b/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol @@ -14,3 +14,32 @@ contract B is A{ used = address(0); } } + +library C { + uint internal constant good = 0x00; // other contract can access this constant + function c() public pure returns (uint){ + return 0; + } + +} + +abstract contract F { + uint private bad1 = 0x00; + uint private good1 = 0x00; + uint internal good2 = 0x00; + function use() external returns (uint){ + return good1; + } +} + +abstract contract H { + uint private good1 = 0x00; + uint internal good2 = 0x00; + uint internal bad1 = 0x00; +} + +contract I is H { + function use2() external returns (uint){ + return good2; + } +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol-0.7.6.zip b/tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol-0.7.6.zip index 2feb3c2f6db0f5494f7686efed23719e46d198ea..13cab662f312bfab7dd45bfd450f397e08ec114f 100644 GIT binary patch delta 2809 zcmV@KL7#%4ghAkidfCpSb$9n005a`kr>&3)x=)1IGUF0{G4}gMYm~Tx#&ms$Hs}phyq%#f_uYwqgZMUN?V# z{{pttjGsX|7Z=BW^Z@UfA$Z$uk||6L>&G<5QukeNvg1j$tZxhKljO`)&<%JV;`x41gYD6WBEWS zrr{$9DeNoW;oX#L6J|%%GdAA*z;tf2!|J?rYH5k#Fd9%qwKqQEXPJ=TTAqY!Yk~3Y z1g+oWkojMKiV$D&v6!CG;f>2;=_Z+5MUvfg_48%&BHMrWNWjX46k?@TUGz~J=~dY& zm2?;!#FERqsBH}T(78bk(yLDNPIrf@ae9p{Wk5|Q?Hn?$FSzf964~DWqK&qRDf#Is zio^OobSdn8dyPRAkopn&2KuGH4=ZF!o3Oln(fo{mXG)T3TrTattKgP{D*-3!+^YAM zEVC6b7N1GFd^hih?^A<1SUoya)8!P|=%){Nx#9{bH_BE^VTp$nC?E6ku<{GaTs)DORvEV)Dy7GI5>WbcQ_(*FvE!S;?rZ3lG?p{v+aXwHfa2aCj z)vZ9US{y_Wv76;0%Hy#kclmwVi=)(zK~U1enNXZ-a#F)w@JE?deB>H4d;i3=)-!mT zV_G@|r@s2#tagv*Vt+aRCRC!^uOPWJjuV{9ciXRdf@MT1> zg4g&S*RT84gef^>qzYfC4;95~`c+U61CIhy%Qm>Z#pX@N_h<1$yEyu$I#q-K`g62% z_ikA43Iw@%<*$F7-wwA*3Z-l>`Dg^+=YYVtGPKCd!#o+e``p>*_F7HphCbtek4TI; z{JAQ_+FdS50~SP?kIr@>m^%4A^zx|3#rzB&aEsH&9*gBg91LZ#Z_?qXIkdiR|HB%USa2(KxIVxx# zFT)a9i#H51jpGmoO5dR05BBl89EVY@7+(peA7)8JLmMm`(3N6e4(db;(8T@%s zrTZAt)$l=48YW!1e58C>iV#kxd2e(5zIcaPX%t_0#A6-8ZL}e#g{jK0iDL5Y|u2g z+jXel#Qb60p-fZN9O3<~%O6>-o|74)3Zp??ibEIEd6qygI!h5O!c8QFC*tM(|BW{5fnz_4B<#^PMMeG< z(;X-OUeeUse4sn+kf(wAO?!#NMAkU5bL*qIrIZHqbJr2M{f-BJoP)&TcA(K{XVz?G zRyoIn9OcIvc42vj0U$78(g>zCnp;|9y5=6}|>wU;LwzPj{t^N8b#gc4LnRNGoiUD>p|-*14NRK+}{qJj=2! z^D*W3@Pp6zTt!}hE->GGdMrrQ1Wz$?>z*t6`bAPLt*bwG6(w{(snhqsAxhHmIOwVX zB_6N${oA3hOpdcVfLmxwfg#2=%liP!lmZ|JCDm?!YL%yd`-#)0NAm4b5|_edsxsH? zNC;9NtuKuUBB-jPp=YLFNh;n)4G++?Kj``~C*eYbS93hO`)b8ni+@`rYi#KWdL27n zNnKr^O?myyd}BdARySuKI#hSk-Liuy98gE?2ewHFzz?bgs6Hu)``3!T*#Bt_>TzX} z8De5ZoDb-K&;BNT_Fiq?$Z_I4Cfpz_Nl>#ZTszu0bVE%n4DE;y6rQg0#(z_KZ-C(o z>n+!9_NJ}pX2Zf{8nkA=wT?^wNwqIt8F3HP>Jgs0Vitubtvui;Iu(cx{ONE7R1f86 zJXIu24;meX1?R(OnLvDD)0KCbS@(|!_ZrOk+6Wu(g)v+De)d|Qfm}UE1(?x_Za9moI&AhRPwV>oM;O&nC{K!_o zAA+}kwmAc66Ft+#5nHTzIk`LdnD+jSx+ZC=@_oARE-X8VS-|k{P;gwX&nJRNEk(t# zNJ_9L0_k)$Q(0BiR_~*`q8-QO|9I&*nuw9Hwq6UBm-lb@*AHygV6MY>Vo8-269DXK z>>$Q;4T&F+KFZqqzcOWD*MdQKXSmh;{Q!Y~n9!eE(fvH|W9wWaa|mW~*55qn-$AWz zJ8<7vBZndF4qD2YpyPfXR8cq4%VS@ME7*Z9)a{?oW4TUw>=S7~M_*&${5M!Lzr=xm z@i-5a1)*93VvaN(tRaIUED#(eblB|KV@Q>_2@rQyG>1=Q~$eiZ%9U*@~S{`p03tum|4sn z45EGoVYjtsQc%{9CWt8Y#*IvvBtvJujiIFi+OVu7cH!=$0{&i82)F#zuxx;Zsd%g+W)f`TrWUGR+a(q^`6Jlp&aLElM`@8zAzr!1X) zW^ps=sptYvWyPd1lA^iz=HLJ6JbznIO928u13v%)01g0Vxr$iL*I0l}3jhF_VUq+0 LNCuA!00000J8Xb$ delta 1611 zcmV-R2DJJ27t#(GP)h>@KL7#%4gfZdcUG7M#=@uu008qOkr>&3(ofCm^m@c&vm#yh z)Dc#cwt7HfVvRu6z)F-=kkID=>(WIWeY-0ie!>i<>UMw{?**&tyf6jI+FfWf1{|0_ac~ z52N*)f`#X$wtJy}qd5RnbzY~~wVVFbv?Lts8NZ!j9J7nn=b!BlD^^IcoVH!>;_e*G zb5kqfK8Q3xa$l4=A1om|a3)ebA`iP6YkCy^N1>6gW@(iUBL@t(g8Gx>Uj8-x4Mx8! z1#kI(6>8V18+b@FO<%^q-|ZW3E}<#A!&{;3)nm1|!Pu{V=wkU~+YaCOEm}+~90EHr zaQ_K$*<}r}ySBBLrAffb@5G~6+S+Cz*lQnZTka@jwwumRv0fX=Ns=`Iy}V`lily+t zYtLtnad(Y*xEVtY;IW+^tJovFAQ&8Q%3o36j%}ik$6WD7)owCs;d}SCf0J_+^RAw!KFN>V_??9XtL zgT)TTH@G-do=%>J%l_}A;|yoXPz?JEI^F>vSj}6eMiI!24dQX|J`@K*ZD_1S;pt+D zwX5|1ODaj&OqAfk!?|u0L~#3~cOy&_^pu>EcCfR5TCIst##-U6q4fNIc^%`)t~A4n z5-fIBrm6g3+0r7F-otLli1l3AoAcfzowVM?W(^HNcN}`_RfRxrgYklWFr4EA0sEPd zZ^c4sI*XS5l7UX^^!3w7{P5WaXMtL{GeO?KL+nSS#qIn$ABIZzYE5@P^e5bynCE1Pfiyr0=(r;_ zMhWA%RpG>ni$*_&Vl|U9q}^u5-3K#@U7)&u>j4-J_xA3WJVD(Gk8hSE9xp2ibh(^$ z&tCS;&|CLrk_^LCK#`%W7<@LTs3Y()@RDM|2Z#xCGp@sPF{Ot6+p0l4nU7x=ZKxuH^74isCNRG}{n3L_ECI>Y)3Eb_I*8IoRVaB@+s1tuPopw_ z)gISoDuNa7%NoUUcZZEkaacSN@2z6RhMpNN{1%Ni;-lZDPiP;`Oy7>}8W`vc$q1vj za(u?hqx@s~X_5x#Ot1qV1e1uT_8=K5z}Lh(HmU#}`z)pLR|}^!AWo|E)u2mz&{Q2Z8xb?;azht&c>1ko||SL-Tvz+=hgWB#97F*slkazpL?eify;c zYe{}Wlc;m?E**EMifEc!jg3R3c#`q1ql>MyhR6HA_Vip1S>16u;N<~gCu&ku)! z8=wU{fc4?QtzCe;{PSZ9(?tJ0ET@~H5}_52G-XotejUB==a^ib$1Rj8a|&yJruUO> zTLITW`7^-KZK|x#uqX~fx9X1~3Tl0>LbB|m(3U8>h^`f&^~9%zpydk z6;aq(>{0IIT(wW13;r9uTTCAvEojs#bOtMI71-uS0e%gqBAa<0xPynEW-EK}dWG0w zyTp0)(rB{-wfN^sz!JQzuoUorK@mJjfP1n=y0x$tQ0f zzSgWQv|JGtOkH%%L54_^dAENRV#V>+TU#OdHqAWw;i&2MUQ|WsGVeft{X0VKz1C*0 zG~BD)3_CiGBiS=>5-8rqpL`-ltj?21<29HHyAioV~*&8k|H_|7%I|e(5rm) zng$+Y@F{vbzt?y)=4${S|M$7eyHHC30zU&k00ICG05*+xR+t6G!l(uS0P`f1sS8L3 J>IMJ+008e$9Ekt` From 9413e1ea9461e68127510e182a32ac0e8dafbf3d Mon Sep 17 00:00:00 2001 From: Alexis Date: Thu, 18 Apr 2024 10:28:30 +0200 Subject: [PATCH 4/8] Fix #2430 EVM printer now correctly detects missing bytecode in contract and don't try to analyze them. --- slither/printers/summary/evm.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/slither/printers/summary/evm.py b/slither/printers/summary/evm.py index 0e5ce58d7..aad71bdff 100644 --- a/slither/printers/summary/evm.py +++ b/slither/printers/summary/evm.py @@ -1,6 +1,8 @@ """ Module printing evm mapping of the contract """ +import logging + from slither.printers.abstract_printer import AbstractPrinter from slither.analyses.evm import ( generate_source_to_evm_ins_mapping, @@ -9,6 +11,9 @@ from slither.analyses.evm import ( from slither.utils.colors import blue, green, magenta, red +logger: logging.Logger = logging.getLogger("EVMPrinter") + + def _extract_evm_info(slither): """ Extract evm information for all derived contracts using evm_cfg_builder @@ -24,6 +29,16 @@ def _extract_evm_info(slither): contract_bytecode_runtime = contract.file_scope.bytecode_runtime( contract.compilation_unit.crytic_compile_compilation_unit, contract.name ) + + if not contract_bytecode_runtime: + logger.info( + "Contract %s (abstract: %r) has no bytecode runtime, skipping. ", + contract.name, + contract.is_abstract, + ) + evm_info["empty", contract.name] = True + continue + contract_srcmap_runtime = contract.file_scope.srcmap_runtime( contract.compilation_unit.crytic_compile_compilation_unit, contract.name ) @@ -80,6 +95,10 @@ class PrinterEVM(AbstractPrinter): for contract in self.slither.contracts_derived: txt += blue(f"Contract {contract.name}\n") + if evm_info.get(("empty", contract.name), False): + txt += "\tempty contract\n" + continue + contract_file = self.slither.source_code[ contract.source_mapping.filename.absolute ].encode("utf-8") From bb8ad1800123f79005e8166b81d6637ffc1d89ae Mon Sep 17 00:00:00 2001 From: careworry Date: Thu, 18 Apr 2024 19:19:13 +0800 Subject: [PATCH 5/8] chore: fix some typos in comments Signed-off-by: careworry --- slither/core/declarations/function.py | 8 ++++---- slither/detectors/naming_convention/naming_convention.py | 2 +- slither/detectors/statements/divide_before_multiply.py | 4 ++-- slither/slithir/convert.py | 2 +- slither/tools/kspec_coverage/analysis.py | 2 +- tests/unit/slithir/test_ssa_generation.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 958a7d219..be215bd1f 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -1593,7 +1593,7 @@ class Function(SourceMapping, metaclass=ABCMeta): # pylint: disable=too-many-pu write_var = [x for x in write_var if x] write_var = [item for sublist in write_var for item in sublist] write_var = list(set(write_var)) - # Remove dupplicate if they share the same string representation + # Remove duplicate if they share the same string representation write_var = [ next(obj) for i, obj in groupby(sorted(write_var, key=lambda x: str(x)), lambda x: str(x)) @@ -1604,7 +1604,7 @@ class Function(SourceMapping, metaclass=ABCMeta): # pylint: disable=too-many-pu write_var = [x for x in write_var if x] write_var = [item for sublist in write_var for item in sublist] write_var = list(set(write_var)) - # Remove dupplicate if they share the same string representation + # Remove duplicate if they share the same string representation write_var = [ next(obj) for i, obj in groupby(sorted(write_var, key=lambda x: str(x)), lambda x: str(x)) @@ -1614,7 +1614,7 @@ class Function(SourceMapping, metaclass=ABCMeta): # pylint: disable=too-many-pu read_var = [x.variables_read_as_expression for x in self.nodes] read_var = [x for x in read_var if x] read_var = [item for sublist in read_var for item in sublist] - # Remove dupplicate if they share the same string representation + # Remove duplicate if they share the same string representation read_var = [ next(obj) for i, obj in groupby(sorted(read_var, key=lambda x: str(x)), lambda x: str(x)) @@ -1624,7 +1624,7 @@ class Function(SourceMapping, metaclass=ABCMeta): # pylint: disable=too-many-pu read_var = [x.variables_read for x in self.nodes] read_var = [x for x in read_var if x] read_var = [item for sublist in read_var for item in sublist] - # Remove dupplicate if they share the same string representation + # Remove duplicate if they share the same string representation read_var = [ next(obj) for i, obj in groupby(sorted(read_var, key=lambda x: str(x)), lambda x: str(x)) diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 827cfe246..ab43318f1 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -16,7 +16,7 @@ class NamingConvention(AbstractDetector): Exceptions: - Allow constant variables name/symbol/decimals to be lowercase (ERC20) - - Allow '_' at the beggining 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 - Ignore echidna properties (functions with names starting 'echidna_' or 'crytic_' """ diff --git a/slither/detectors/statements/divide_before_multiply.py b/slither/detectors/statements/divide_before_multiply.py index 1f6ccd87e..e33477135 100644 --- a/slither/detectors/statements/divide_before_multiply.py +++ b/slither/detectors/statements/divide_before_multiply.py @@ -80,7 +80,7 @@ def _explore( for ir in node.irs: if isinstance(ir, Assignment): if ir.rvalue in divisions: - # Avoid dupplicate. We dont use set so we keep the order of the nodes + # Avoid duplicate. We dont use set so we keep the order of the nodes if node not in divisions[ir.rvalue]: # type: ignore divisions[ir.lvalue] = divisions[ir.rvalue] + [node] # type: ignore else: @@ -94,7 +94,7 @@ def _explore( nodes = [] for r in mul_arguments: if not isinstance(r, Constant) and (r in divisions): - # Dont add node already present to avoid dupplicate + # Dont add node already present to avoid duplicate # We dont use set to keep the order of the nodes if node in divisions[r]: nodes += [n for n in divisions[r] if n not in nodes] diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index b8ce460bf..7d8aa543b 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -482,7 +482,7 @@ def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> Li call_data.remove(ins.variable) if isinstance(ins, Argument): - # In case of dupplicate arguments we overwrite the value + # In case of duplicate arguments we overwrite the value # This can happen because of addr.call.value(1).value(2) if ins.get_type() in [ArgumentType.GAS]: calls_gas[ins.call_id] = ins.argument diff --git a/slither/tools/kspec_coverage/analysis.py b/slither/tools/kspec_coverage/analysis.py index 763939e25..a2ae23660 100755 --- a/slither/tools/kspec_coverage/analysis.py +++ b/slither/tools/kspec_coverage/analysis.py @@ -59,7 +59,7 @@ def _get_all_covered_kspec_functions(target: str) -> Set[Tuple[str, str]]: def _get_slither_functions( slither: SlitherCompilationUnit, ) -> Dict[Tuple[str, str], Union[FunctionContract, StateVariable]]: - # Use contract == contract_declarer to avoid dupplicate + # Use contract == contract_declarer to avoid duplicate all_functions_declared: List[Union[FunctionContract, StateVariable]] = [ f for f in slither.functions diff --git a/tests/unit/slithir/test_ssa_generation.py b/tests/unit/slithir/test_ssa_generation.py index 1a709c2f7..2d6c9c8c1 100644 --- a/tests/unit/slithir/test_ssa_generation.py +++ b/tests/unit/slithir/test_ssa_generation.py @@ -502,7 +502,7 @@ def test_storage_refers_to(slither_from_solidity_source): When a phi-node is created, ensure refers_to is propagated to the phi-node. Assignments also propagate refers_to. Whenever a ReferenceVariable is the destination of an assignment (e.g. s.v = 10) - below, create additional versions of the variables it refers to record that a a + below, create additional versions of the variables it refers to record that a write was made. In the current implementation, this is referenced by phis. """ source = """ From 2979cb85ac06f1b059bbec8e90e7dbb4a6755d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Fri, 19 Apr 2024 16:34:19 -0300 Subject: [PATCH 6/8] Restore plugin example to working state The current example depended on a very old Slither version, and triggered an error due to its empty description, URL, etc. Add example values so the detector can be run. --- plugin_example/setup.py | 4 ++-- plugin_example/slither_my_plugin/detectors/__init__.py | 0 plugin_example/slither_my_plugin/detectors/example.py | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 plugin_example/slither_my_plugin/detectors/__init__.py diff --git a/plugin_example/setup.py b/plugin_example/setup.py index 1bc065394..908ad51da 100644 --- a/plugin_example/setup.py +++ b/plugin_example/setup.py @@ -1,14 +1,14 @@ from setuptools import setup, find_packages setup( - name="slither-my-plugins", + name="slither_my_plugin", description="This is an example of detectors and printers to Slither.", url="https://github.com/trailofbits/slither-plugins", author="Trail of Bits", version="0.0", packages=find_packages(), python_requires=">=3.8", - install_requires=["slither-analyzer==0.1"], + install_requires=["slither-analyzer>=0.6.0"], entry_points={ "slither_analyzer.plugin": "slither my-plugin=slither_my_plugin:make_plugin", }, diff --git a/plugin_example/slither_my_plugin/detectors/__init__.py b/plugin_example/slither_my_plugin/detectors/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/plugin_example/slither_my_plugin/detectors/example.py b/plugin_example/slither_my_plugin/detectors/example.py index 2f10cb518..edf2d382a 100644 --- a/plugin_example/slither_my_plugin/detectors/example.py +++ b/plugin_example/slither_my_plugin/detectors/example.py @@ -11,12 +11,12 @@ class Example(AbstractDetector): # pylint: disable=too-few-public-methods IMPACT = DetectorClassification.HIGH CONFIDENCE = DetectorClassification.HIGH - WIKI = "" + WIKI = "https://www.example.com/#example-detector" - WIKI_TITLE = "" - WIKI_DESCRIPTION = "" - WIKI_EXPLOIT_SCENARIO = "" - WIKI_RECOMMENDATION = "" + WIKI_TITLE = "example detector" + WIKI_DESCRIPTION = "This is an example detector that always generates a finding" + WIKI_EXPLOIT_SCENARIO = "Scenario goes here" + WIKI_RECOMMENDATION = "Customize the detector" def _detect(self): From 8c46b9ea7c9683e369e9f30f67d30d5347087935 Mon Sep 17 00:00:00 2001 From: Alexis Date: Tue, 23 Apr 2024 09:59:00 +0200 Subject: [PATCH 7/8] Configure coderabbit review to also consider PR on dev branch. --- .coderabbit.yaml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .coderabbit.yaml diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 000000000..c1e63d0c0 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,25 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +language: "en" +early_access: false +knowledge_base: + learnings: + scope: auto + issues: + scope: global +reviews: + profile: "chill" + request_changes_workflow: false + high_level_summary: true + poem: false + review_status: true + collapse_walkthrough: true + auto_review: + enabled: true + ignore_title_keywords: + - "WIP" + - "DO NOT MERGE" + drafts: false + base_branches: + - dev +chat: + auto_reply: true \ No newline at end of file From 7fff11b7251f20159465a73f81f477c01e55d576 Mon Sep 17 00:00:00 2001 From: alwayshang Date: Wed, 24 Apr 2024 17:20:24 +0800 Subject: [PATCH 8/8] chore: fix some typos in comments Signed-off-by: alwayshang --- slither/detectors/compiler_bugs/reused_base_constructor.py | 2 +- slither/solc_parsing/declarations/contract.py | 2 +- tests/unit/core/test_source_mapping.py | 2 +- tests/unit/slithir/test_data/ternary_expressions.sol | 2 +- tests/unit/slithir/test_ssa_generation.py | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/slither/detectors/compiler_bugs/reused_base_constructor.py b/slither/detectors/compiler_bugs/reused_base_constructor.py index 73bd410c7..4764dc641 100644 --- a/slither/detectors/compiler_bugs/reused_base_constructor.py +++ b/slither/detectors/compiler_bugs/reused_base_constructor.py @@ -35,7 +35,7 @@ class ReusedBaseConstructor(AbstractDetector): ARGUMENT = "reused-constructor" HELP = "Reused base constructor" IMPACT = DetectorClassification.MEDIUM - # The confidence is medium, because prior Solidity 0.4.22, we cant differentiate + # The confidence is medium, because prior Solidity 0.4.22, we can't differentiate # contract C is A() { # to # contract C is A { diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index ebab0e6df..1ccdc5760 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -593,7 +593,7 @@ class ContractSolc(CallerContextExpression): def analyze_constant_state_variables(self) -> None: for var_parser in self._variables_parser: if var_parser.underlying_variable.is_constant: - # cant parse constant expression based on function calls + # can't parse constant expression based on function calls try: var_parser.analyze(self) except (VariableNotFound, KeyError) as e: diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index 298a192d5..2a1bbc551 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -58,7 +58,7 @@ def test_source_mapping_inheritance(solc_binary_path, solc_version): assert {(x.start, x.end) for x in slither.offset_to_implementations(file, 203)} == {(193, 230)} # Offset 93 is the call to f() in A.test() - # This can lead to three differents functions, depending on the current contract's context + # This can lead to three different functions, depending on the current contract's context functions = slither.offset_to_objects(file, 93) print(functions) assert len(functions) == 3 diff --git a/tests/unit/slithir/test_data/ternary_expressions.sol b/tests/unit/slithir/test_data/ternary_expressions.sol index 1ccd51d34..43ba9b9cb 100644 --- a/tests/unit/slithir/test_data/ternary_expressions.sol +++ b/tests/unit/slithir/test_data/ternary_expressions.sol @@ -6,7 +6,7 @@ contract C { // TODO // 1) support variable declarations //uint min = 1 > 0 ? 1 : 2; - // 2) suppory ternary index range access + // 2) support ternary index range access // function e(bool cond, bytes calldata x) external { // bytes memory a = x[cond ? 1 : 2 :]; // } diff --git a/tests/unit/slithir/test_ssa_generation.py b/tests/unit/slithir/test_ssa_generation.py index 1a709c2f7..1358fa957 100644 --- a/tests/unit/slithir/test_ssa_generation.py +++ b/tests/unit/slithir/test_ssa_generation.py @@ -502,7 +502,7 @@ def test_storage_refers_to(slither_from_solidity_source): When a phi-node is created, ensure refers_to is propagated to the phi-node. Assignments also propagate refers_to. Whenever a ReferenceVariable is the destination of an assignment (e.g. s.v = 10) - below, create additional versions of the variables it refers to record that a a + below, create additional versions of the variables it refers to record that a write was made. In the current implementation, this is referenced by phis. """ source = """ @@ -550,7 +550,7 @@ def test_storage_refers_to(slither_from_solidity_source): entryphi = [x for x in phinodes if x.lvalue in sphi.lvalue.refers_to] assert len(entryphi) == 2 - # The remaining two phis are the ones recording that write through ReferenceVariable occured + # The remaining two phis are the ones recording that write through ReferenceVariable occurred for ephi in entryphi: phinodes.remove(ephi) phinodes.remove(sphi)