From b59cf81c9e038d54482fcd6b0b8e4c74fd08d92a Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Thu, 20 Feb 2025 11:57:31 +0100 Subject: [PATCH 1/4] Improve the support for sstore/sload with simple slot access During the IR generation, convert the sstore/sload into direct IR assignement when we have a state_variable.slot direct access This allow the SSA to be generated correctly, and for the var read/written analyses to use the right variables. Fix #2642 #2491 #2470 #2160. Replace #2669 Revert #2329 --- slither/core/cfg/node.py | 16 ---- .../visitors/slithir/expression_to_slithir.py | 19 +++++ ...ouldBeConstant_0_8_0_unused_yul_sol__0.txt | 0 .../constable-states/0.8.0/unused_yul.sol | 60 ++++++++++++++ .../0.8.0/unused_yul.sol-0.8.0.zip | Bin 0 -> 4446 bytes tests/e2e/detectors/test_detectors.py | 5 ++ tests/e2e/solc_parsing/test_ast_parsing.py | 1 + .../compile/yul-solady.sol-0.8.27-compact.zip | Bin 0 -> 5144 bytes .../yul-solady.sol-0.8.27-compact.json | 9 +++ .../e2e/solc_parsing/test_data/yul-solady.sol | 75 ++++++++++++++++++ .../test_data/assembly_sstore_sload.sol | 29 +++++++ .../slithir/test_yul_parser_assembly_slot.py | 26 ++++++ 12 files changed, 224 insertions(+), 16 deletions(-) create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_CouldBeConstant_0_8_0_unused_yul_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol create mode 100644 tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol-0.8.0.zip create mode 100644 tests/e2e/solc_parsing/test_data/compile/yul-solady.sol-0.8.27-compact.zip create mode 100644 tests/e2e/solc_parsing/test_data/expected/yul-solady.sol-0.8.27-compact.json create mode 100644 tests/e2e/solc_parsing/test_data/yul-solady.sol create mode 100644 tests/unit/slithir/test_data/assembly_sstore_sload.sol diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index fc178db4a7..83c896a42e 100644 --- a/slither/core/cfg/node.py +++ b/slither/core/cfg/node.py @@ -11,7 +11,6 @@ SolidityFunction, ) from slither.core.expressions.expression import Expression -from slither.core.expressions import CallExpression, Identifier, AssignmentOperation from slither.core.solidity_types import ElementaryType from slither.core.source_mapping.source_mapping import SourceMapping from slither.core.variables.local_variable import LocalVariable @@ -889,21 +888,6 @@ def _find_read_write_call(self) -> None: # pylint: disable=too-many-statements # TODO: consider removing dependancy of solidity_call to internal_call self._solidity_calls.append(ir) self._internal_calls.append(ir) - if ( - isinstance(ir, SolidityCall) - and ir.function == SolidityFunction("sstore(uint256,uint256)") - and isinstance(ir.node.expression, CallExpression) - and isinstance(ir.node.expression.arguments[0], Identifier) - ): - self._vars_written.append(ir.arguments[0]) - if ( - isinstance(ir, SolidityCall) - and ir.function == SolidityFunction("sload(uint256)") - and isinstance(ir.node.expression, AssignmentOperation) - and isinstance(ir.node.expression.expression_right, CallExpression) - and isinstance(ir.node.expression.expression_right.arguments[0], Identifier) - ): - self._vars_read.append(ir.arguments[0]) if isinstance(ir, LowLevelCall): assert isinstance(ir.destination, (Variable, SolidityVariable)) self._low_level_calls.append(ir) diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 1d68336bd6..2a132339cb 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -406,6 +406,25 @@ def _post_call_expression(self, expression: CallExpression) -> None: self._result.append(var) set_val(expression, val) + elif (called.name == "sload(uint256)" or called.name == "sstore(uint256,uint256)") and (len(args)>0 and isinstance(args[0], StateVariable)): + # parse_yul._parse_yul_magic_suffixes does a best effort tentative to retrieve + # the right state variable on .slot access + # + # Solidity does not allow state variable to be directly used through sstore/sload + # As you need to specify the slot number (ex you can't do " sload(some_state_variable)") + # + # So we can make the assumption that if a state variable appear on the first argument + # of sstore/sload, we can convert the call to sstore to a normal assignment / read + + if called.name == "sload(uint256)": + val = TemporaryVariable(self._node) + var = Assignment(val, args[0], ElementaryType("uint256")) + self._result.append(var) + set_val(expression, val) + else: + var = Assignment(args[0], args[1], ElementaryType("uint256")) + self._result.append(var) + set_val(expression, args[0]) else: # If tuple if expression.type_call.startswith("tuple(") and expression.type_call != "tuple()": diff --git a/tests/e2e/detectors/snapshots/detectors__detector_CouldBeConstant_0_8_0_unused_yul_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_CouldBeConstant_0_8_0_unused_yul_sol__0.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol b/tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol new file mode 100644 index 0000000000..6eb4ca98cb --- /dev/null +++ b/tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol @@ -0,0 +1,60 @@ +// adapted from https://github.com/crytic/slither/issues/2325 + +contract L1Block { + + address internal constant cDEPOSITOR_ACCOUNT = 0xDeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001; + + /// @notice Address of the special depositor account. + function DEPOSITOR_ACCOUNT() public pure returns (address addr_) { + addr_ = cDEPOSITOR_ACCOUNT; + } + + /// @notice The latest L1 block number known by the L2 system. + uint64 public number; + + /// @notice The latest L1 base fee. + uint256 public basefee; + + /// @notice The latest L1 blockhash. + bytes32 public hash; + + /// @notice The number of L2 blocks in the same epoch. + uint64 public sequenceNumber; + + /// @notice The versioned hash to authenticate the batcher by. + bytes32 public batcherHash; + + /// @notice The latest L1 blob base fee. + uint256 public blobBaseFee; + + /// @notice Updates the L1 block values for an Ecotone upgraded chain. + /// Params are packed and passed in as raw msg.data instead of ABI to reduce calldata size. + /// Params are expected to be in the following order: + /// 1. _baseFeeScalar L1 base fee scalar + /// 2. _blobBaseFeeScalar L1 blob base fee scalar + /// 3. _sequenceNumber Number of L2 blocks since epoch start. + /// 4. _timestamp L1 timestamp. + /// 5. _number L1 blocknumber. + /// 6. _basefee L1 base fee. + /// 7. _blobBaseFee L1 blob base fee. + /// 8. _hash L1 blockhash. + /// 9. _batcherHash Versioned hash to authenticate batcher by. + function _setL1BlockValuesEcotone() internal { + address depositor = DEPOSITOR_ACCOUNT(); + assembly { + // Revert if the caller is not the depositor account. + if xor(caller(), depositor) { + mstore(0x00, 0x3cc50b45) // 0x3cc50b45 is the 4-byte selector of "NotDepositor()" + revert(0x1C, 0x04) // returns the stored 4-byte selector from above + } + // sequencenum (uint64), blobBaseFeeScalar (uint32), baseFeeScalar (uint32) + sstore(sequenceNumber.slot, shr(128, calldataload(4))) + // number (uint64) and timestamp (uint64) + sstore(number.slot, shr(128, calldataload(20))) + sstore(basefee.slot, calldataload(36)) // uint256 + sstore(blobBaseFee.slot, calldataload(68)) // uint256 + sstore(hash.slot, calldataload(100)) // bytes32 + sstore(batcherHash.slot, calldataload(132)) // bytes32 + } + } +} \ No newline at end of file diff --git a/tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol-0.8.0.zip b/tests/e2e/detectors/test_data/constable-states/0.8.0/unused_yul.sol-0.8.0.zip new file mode 100644 index 0000000000000000000000000000000000000000..c3d44f0c11836b859f2d8a80c44597060ada0312 GIT binary patch literal 4446 zcmai&Ra_Gez_mw7*Fa))Nq3GO!szaj?(Xg!Ey7@cG$}zL~*(l{+{r~vG9*ggqGKs4+z?&Z6cW@35J`AZy#;L_fvN1>^9;YTQnqu{G}K zPxr~0)KN+qy}x?A_a$9?1D_<@B$2h26n7QTs;4$GN9?KsceK_Kb6dOt$B>@WahnW+ zpW8vuh!9hXRH5MwJkgw9(87d;GqVjPp!oOK&Pks7U$=nuYEa^Q|oWQ~Us`y*y zRzb~tZX^^n2*TCdoDn093uozU4J#$|1c>1&LQ+_IY|TI%{~8a_w6&~J>lRXTa6nlE@H4g9bd0>m3ompl6~!xK@TDlStjlPC+`=tAK#3SikN1PPScVf zqdNJba2?F;G-T8w1~+p1_ycM3O|K4)*O*zRThWDDj_m zlW2P9ll-5#&?>=nZNS!Yr=d!tnQ%^d+4%DMBDUN4*5r~)_Bhox z^8Puc%X)@()g)>)+%Jb-TwR#re7c$9^GRq zM0JT-r*ylWvTw4G^iF}eDVoJW^zwnKsg`(zYZ3ol#&!^kP%Qf&Pa$}^t{=ZIdBi!h z8G$wi_I?!axJ1BWV?&10j{zO-+2L*>J9>`O_KmbvPL%~VGS3!g*gFju^vX-J#$RvE z$a4R@@x(eNf9Mx0R+vZovt=ft5j9cc9Z zoksXd)cQR9)#VWsG=F16?U*?xq?rysNl_bgYfMxtWfc%~uyRIFh2n3O@zzK{{smcd zMe^X%EHHkpxx`N_U0G7)N(e9q#bOK#R2oM_aw@YtPeIvXm>gLRp#j|NXj7-Yr*+Q} zr16~=C&3QEcK8$vB(?q~Ntxxhr9J*+pS0Z#J@i)WDE7{muJ%zW)=`mDafLJKnWwqb z-8#$Zffm3pDe3FSAL}=BH^$0xc)w_mh8&a zw0~oT$W&%GJ}5=7;w*A%v`%9iHF_PtYQvYMkyChB9}N^aEsN7qG4u^hsPKub28eQ7 zl(Bz-Q7rmR{~YjbiQG!P@wog{9?$#Mp(aiVATYB%!^C}ea~Fb98(YI?EV&SCa7Sq@ z5_#hnoAUV9{td6FcMvP1#5cB(;JM;*)0HWqtHh$QEKA*0o-c-6!Um$P99&((WO8iZ z6XUtpd`MkcM%bl30!J#W>!0-m(HxqXj0UoM>N!nI1~G5cX`sO^S2X*erGpRqC3?!)*oPu3Ac3X&cRwm zLU3bRr1?X`UtT~q7_D1s3&g` zssrS`h(6dnlhMs;)ksGmI^)aJ&HNPo25%#Q+-PIuOG zp;>pwc2SYz2QcU0mkuB&d=RO8h0PDK?O9(l(?R@uFfI8vVvtZc|=s_WC+s)oD437kRxpc=jb0`>5dK?u)FBhxK24ZN=)ERMS%4zHWyYgh! zm~C;&)%TVOnK%Vz0z14g^D|{aZt|h=3qD>s8y)XyQi!fp&0{JK4^sWNIKzJL(2|xb zuod3Ce0LoTMB~l&6-s-mRjqz^BYujq14FkBQoIY{xSaUFkxXo-T{@jqG)7)vD z`SY)vjWS4B>1B+wf0G``)o;DCx!oZC2ma0-N3IAL5A~X=nrQ@Gw~Nl}wOdRfsH^cK zRk6wfIPtEB_J%EC1ZPzvqLeuA3Q=lj7vnfFG_3P1go^r_6Lo$8--~<;cWILoufUzo zR<=SzApELXb5rQSFNRi_iPv{nPjN(XU(G8BL;2lOkgk)nj2)YUs|wL@c|K|rqZJ=Y z!>|LAt==Tc$0g%TEq6KA=P!ax>lH~AgbITHDBUezI?`|Y_+|YClT0Uq{xo<>- zPd$He$Ws)}osdsz*t&khDXT)nUWX*K)p6K?+!Lx$!=Av>?fw{;=AHLIyu6PycCn%Q zxCE4gL}Za+s{aPTL1%wVlqIF6W}xEkR<hA_7LWN5k~x#;ib} zbfO28gq3XH$6(Av{pdxe;x#&Y3YJ%__NQ$n^76TJuE8F42knK9SMf)%QmFg}?=K+Y z@cN97Ap?QZT~|WlHMa%Dj2C4aRz|A#7gHm*j$ISXrdkJhXbM&{&cc~gHytlag<_l< z{TPMPnZgB}h$bP(XiUhnlYK=t-E#8{mn)Xd?+C5b^s^LUAetXq5!IQF*I*~(tq2wy-*0-t%qi(b zF{D{GtT_Ktg~pbo9@~q(>k6=@`p9RAbwK4viT01v!x$8$JHo9Z^GCdZf-V1n%Nk82 z6fQV?Iz3U{)YyD+ReLxP^sIuW#l#p=b*Li$Eq^D*$082h6s5&F=+{>W{U!%wX%U(c zJ*saW7RF&>$i#2!#}2iNk#yd%}0 z=jPnSwh2sIwinx&C9pp}&D2@#CVEFkZs(O7s5op|Xzo}5AM5Velh0jLEuv_X>%Pnu z98M6`gAu;YR!FyvapUvk3117vWsTZ5 z>FA2g+G8;dE8ID?gdx4Xip4w}0jX1)`M`1a4hx5Rcx63{lDY=DCi~@}AYTS+fWbeV z2C1LOo`gjnuD%{w+XpfoarGy(dn!vTqx|!0Q zN|uN5bAxa%f*a>iWGE;(YQ~G@Rp;kR$DKg2j8*DVRplpJzf3WlZksJ;*e+K1{^Egx z>!kQ7B|^YB`r@ls-GGLH6kq5CP<8sgf;NGLR%6s z8B@w9$Q|Zup{qO@fp) z4pvKw zTi%3Gh%~ip=0+#%iI049)SS)19f>VYO9)@pt+$*R(a!Tfj+7oGlU-i!s|E z+upUrBB@q(gO|+Ee(DR*ZjiG4uu+*i2eyR?OyOzYcC2Lh9N4P%8kNdU1jv^j>3$~* z8*{d^IU(SvpGuqn-3BtjCUoR_gKcjQV& zZzO9Yzd9~AylNA#X`7&jnU-fNcbiF#?%_S30Q>9#L($7N0jmTijf|^$Cg23NmA_&T z?!%X~tN!mV(uU$5*p{p~oxDG~3yuVdeT?JDFi@q~E^SeX)}1k$?D1-vGCCv>!un`F zF(jcWe^+AzraD_>~h3w8AS~lpKM;+GgSD+g zkhX7xcc%WbjHH(Df`7@lVTmKy|Dr7?c=GQ#{x7 List[str]: Test("scope/inherited_function_scope.sol", ["0.8.24"]), Test("using_for_global_user_defined_operator_1.sol", ["0.8.24"]), Test("require-error.sol", ["0.8.27"]), + Test("yul-solady.sol", ["0.8.27"]), ] # create the output folder if needed try: diff --git a/tests/e2e/solc_parsing/test_data/compile/yul-solady.sol-0.8.27-compact.zip b/tests/e2e/solc_parsing/test_data/compile/yul-solady.sol-0.8.27-compact.zip new file mode 100644 index 0000000000000000000000000000000000000000..849144170f733ca1e6f723187c16bd5278aac3db GIT binary patch literal 5144 zcmajjRa+DcxVGV;OS*<0x*O@1F6kOt2Bd39N$D;DK{}+7Zs~4@?(URszIT25>sZgT zaqZl{;8a&eK$HN$1F!(4+S-PEx(2^p@d1GC7yy6<004Nq+q1hl*_&H>bNm-mh?ldI zE7a87^~+Zq4+w{?o0B6NA`-wD00;yC#3CZBd0%*4@?aB(aeb3Mss^?<}Op{hm-h*&T z4xG$S*B0HC%$|9^`^dqc*>Y$h%{I96GR4vnOTS>`%~%N~tRe8DK+5(LShT+Y@ki3d z5n6Yrhp@?aL!CztbB&+nsOH$c~%O+vw9VYBu~d#;Mq1c{;QrQT*W14J*!Jz@syjK0_dV#5cgJ6-gy9~|-DD!4qB58K^Cqug5f8oBolwN~yWVIK4g6BZ9EQ|k z4I$3sV~!VY2(K$Q9uvGn96#8Q@w^|)gwv^~L)_j*Kv|?>6*ZqD&?@a);uHK z>Fyvq70PbdPg0c5ZZ@+@;{VG)RPp2EY|*y?R-Y9sBQ^d4Qly;u>xmOfZs)Cy7a1}f zd{HfP#7cOD?R&yK5@bhrnP2V_%GtlPbcy)|zovelR6cpg6z&`ub#mMR@5kv-vuziW zKxvy*%|0+s_IS_VVB&U2L?O+$>PHH&*@Z^r>0rxCt|IVUpTpVEmS zioXDUTfcy@HdgKef#o!v#JRu#48RIqZt*JLK%2lO%V0vflt^M><#rd(u;fbI)AiH! zE6+0z*$Ke_O*$aV+$rosR_#q!al#O`5p|88dv|UcOfcMy+5Norn3Y>w`vspsThvkF z)5;+~CW;IFY53#Vz;h$JkR{`@)55Y^yFkwj<>OWW>G!=Hkc`EBfnTLx*xZ0%201U6 znAd6Zi;}55%esEHno!XVhp|=mj&5Q7cO&inE#1_K*G(1iiv$HHn-qhMIJT>M(r08} zTlm7L2oX-zFE19gMfV_Rl)2Ql`U=EqVC)p|IK%qCd`v&xsD0Tk0C9B_~0++CYykvn_mkZx- zYeO#BGE}c*1+V8l^v8@3{mDO4=wzjulL>{BEj$=15Y|xxPX%IgG-~^-j$jeGGuIZf zpOWU7(?s>kDn3b+kpB@yCH^&3JIxFmK!l>V8|@9MtmrymZrg$3)+1ocvR{Cya?O|W z1!(IK%+~UZ8;5qC2R>E*unUvx1!_~06%rSBn!o%Pay^#G(UG(jKnCgSz->(c2gK98 zmvGs015a_E;EEGttlQ>YA4jcnwdXN2LXZK$YOEHr}~ov;vUvs*gtB)sAUl#Ee%y?uuyfbATcaIrQ%Z;V685IJA+{AF|h z2*1a|qC(6g^(Rs!Zd@rJs3K)Jqxu&o)IRB=m)D^)lBFe0acYOTnUCa^9v&+R`!4#X zLWg-Im|s3sTNiuZu;?6_Murej*tmA%v@-^ajaOKo5Tf4;$Y|_^BrAbEP-zieD1lJx ztP^&?*qL1UdiXOIAb+_$6AG>7!R7P&JSHEiPpb2pyd!osTUPZji52f1m9D|Ge}iOv&DiFeS$#0?N~g2>{zPAl zo{(tKuem}rE>b}>AMIrniQ@e6nyyg!j_KDKBChWv?`bTHEtwEGq%wAoq%5&Py)YeX84I35! zA=yzUi5^!h+euDp5!dEEe`FIrVEbLz|5)-$LzYsNCM}j|$a3s4WZ*f&af}2hU%nZH zoB26o^xc&>d9f#}1>5$F0mNgY$(!N^u|4yBhK_&no|ek-*L?A_gO{)C_==^HH627% zXP(_)dGZsw=RabvTBb_9KCWOiFjbA&9|p8ntC=Bqr{yiHXba3&L;T7VGONP^iM} zaiwEhuSIvgJ9Ih6gL8yHDw7B{RR&OR^uQ_}j8ZA@2S)3)ty$bTjUV`ZsDrR!OQV2k zA-?&=ph-J?+~fnMl+PIK@-n8Q(O|yQG=EHg)0OXya{n~V&zSW#kJ&EW)2nMlO(HvAf#itNANe* zUPj)gx#q9n=_oo0Tm^{nXtFrG)7)3DABFN}cZyhpf_R-V?Qp;hAE&g8cBoAopO2mm zPgCx#>*P?Dulr)Gg|Qe7lf!v07z$f}k^`z|q?vmkT6)qf3ItbR?5*IEsiq1m)vT%1 zz>WIWG)b3QBhL&ivshdCXu0ua4K);#uA7&u^bK_JZI^t=IwkW{LqOkHzcEg0;Gk0u;@+^5b?x5|I|L07h7LdN~g?~nPI)S% zlRWmfpfm+_9)Ws@M|4G;^KzJD14ne+Aq+FkO69H`v$zb=$%s_AXq_=L!_SsIt|5jy z#_;7xDo~&<6pY3=D$ci6)%_&*{yE6TUBanGC`~>7sE^BZdVw-G(W`&9Y?@n4)C1`g zq6P+Ce{GP}jKcs1Q@dYw4bv9g?@p8$p7|Nc!PNEYjBIl@cqyudzqk2X88$(m5*+MN zsi`8ftofp@EgMPxEsHG~zdFcpnn^ zYHV8@)Hm~L*HTfGlTc0^M>DtbAn5DUxcO(R2yktAf$)sEU10(3KMn0~=cQy))y$v{ zoO^|_wpn@n6zO_rF*$a&ncm!OE>Su}MS8!IMgINMzrEWmK$JKV7z?=mZ3?`rVh}zs zZrSBb#jpg^JpxHP{aubWlo7H7$c1^U7g-SzWv|VvS)umc#)DM=2JX8sc z?;V6v1G*3Ks|u=-*)&IcR%Cn{I+*Rge!cj~1M>H>Qnoo=*yC}ZHVl(O&LO{dK<^4y*m_l`0%StgydQpy_j{9XL zzt;r+iC12hDzOvvW86CagZyONj`agQJ#Wl1eGO$_#HMx(YOwaLx_wL?#Gk~}5yF9> z_*Pe3CwaL%e^Jq4iY7_5ehpB^Lw?+~5b~2dPaLmyAg1OOzkN?mY6q(BA%KSi@t~d( zi%)aMMs}Ch0R~r{dC4ff7Qa`Z?+k~Zx$VL%8^D7KvNyp~=Bd4|f^rUv+!*d8dS>d! z2@`LfL>!Y{f8M$UXPu+#(T51PwBKBW|M9NHDV=1|_&pOpJkb$PjxzP|-pI6ll`~2V zD#kZxl_BIYaLjSUATlBn(78@``|g^+4ea2p-9|$xclBu!jHx}u5p!UbeAMGFxB(qF zGOGkPTuJCRsFFUFqGZde869%Iz|TSnl$&YU3$sa1c`Yc$RfDL9=XTt!)Jw zl6=2<^=$Lv?9PW>8jKv_`$#ZfK8+qp)N;?7aS}b}Skgy`s+iw_T7o4gOPjSCdqj7g z6TV~@+HW}+=xl_;YfT$Q|3v!seLnTxF(s+SsG%+XSAya*gLA>yPH>-tO%voUB$FV< zUQIy=S9@8sZaN6Lw-}^|4$S9Fbz9ps>P8*b3D>n1AHyRM*Axkg{K^swy6#e47)nko zJ=nff{)q^RqBM9Be!9>(-thukyB-kD7cM}xS{|uuwB#PLe0xS!2Sjr}8ceUVolmd1 z5Ub~~wi%V1yP*{!hp>&irrAiL_iNuTmR_2ixAiTBZ`pBWYxESy-LU0v*M4-_mQ`5R ziAnZ~bl6;Hky`(t`TM!t&ap}~+O{UyI%VQ)2FmE^)Zvp0AOE_8;B22t$!u%6DbA>2 zIcX)dr(l zu)S&_-9Hut+DLBw9uK3r6!r{ZVs76MhP2F9qhf>~CT6jrWo{>B1?|JW=kZoCP5G)tFu1gM`i_mb{*>P3@u=v;)#|w^V2HA zC0)e47rwd5Tb~DDUqdoRw|BkZNf##MG6BvlGM1Ag&e|zI^ z>|hv{Pj!Twf&^Yug3SQ+wfy@IJ^O)xIN;&bVH1aVxm{}`qaX@RJj1CeF9cudNAGz# zwz@4s++y&;5*}`4radNj>=9G=^(BUYc=a_?2)x8l@{ra^ro)F}B^?3s38 zgCXpl#VwA%H!+rgUm~FO#bLx(=)%-gh^6X{oi0nce%Cb4r%ca~hmA0#YYgAKh!&c~ z)^^Be5*{Tfoqso-X-Lp(cC`Irhrp#px_7IX{9B4~6B)1or{4Ill@swT`QZKICT3|s z>ea#}b&{>>aLCQ?iYGaRUfgJnxSNL}`DC{RP=-fZZ6GE_*j( zn7W&6;UuGJ)mY*gocxQbg2>22J>scITF~K4=pi2(O`n6Jiqd??HLUg69`a8m*&Wk_ zZ<12!4|Y0tmrB%$3!ES68`>!Y({(a13OOba7XDxzpqX-fbN!KabS#J_X&uM%RqFIb zj>%k^sjK`nL0L|Yj{b(O`nl0!p{jV87%EK|s(PHZtbOI2=TwQX&wLTg!W*%-@^dR< zN0WAOs%eq(ruq#NAPa&h$IWYyC%6=geYC}UFQzi>I#EWbBDc|9+W$8PA_iRj!EW%3D!a qhJ%+t_}|U$Keqe-7YO%1`M(8ET^R}af0l6nz50Kx{m=gZfd21;\n1[label=\"Node Type: INLINE ASM 1\n\"];\n1->2;\n2[label=\"Node Type: NEW VARIABLE 2\n\"];\n2->3;\n3[label=\"Node Type: EXPRESSION 3\n\"];\n3->4;\n4[label=\"Node Type: BEGIN_LOOP 4\n\"];\n4->6;\n5[label=\"Node Type: END_LOOP 5\n\"];\n5->25;\n6[label=\"Node Type: NEW VARIABLE 6\n\"];\n6->7;\n7[label=\"Node Type: EXPRESSION 7\n\"];\n7->8;\n8[label=\"Node Type: IF_LOOP 8\n\"];\n8->5[label=\"True\"];\n8->9[label=\"False\"];\n9[label=\"Node Type: IF 9\n\"];\n9->11[label=\"True\"];\n9->10[label=\"False\"];\n10[label=\"Node Type: END_IF 10\n\"];\n10->23;\n11[label=\"Node Type: IF 11\n\"];\n11->13[label=\"True\"];\n11->12[label=\"False\"];\n12[label=\"Node Type: END_IF 12\n\"];\n12->15;\n13[label=\"Node Type: EXPRESSION 13\n\"];\n13->14;\n14[label=\"Node Type: EXPRESSION 14\n\"];\n14->12;\n15[label=\"Node Type: EXPRESSION 15\n\"];\n15->16;\n16[label=\"Node Type: IF 16\n\"];\n16->18[label=\"True\"];\n16->17[label=\"False\"];\n17[label=\"Node Type: END_IF 17\n\"];\n17->20;\n18[label=\"Node Type: EXPRESSION 18\n\"];\n18->19;\n19[label=\"Node Type: BREAK 19\n\"];\n19->17;\n20[label=\"Node Type: EXPRESSION 20\n\"];\n20->21;\n21[label=\"Node Type: EXPRESSION 21\n\"];\n21->22;\n22[label=\"Node Type: BREAK 22\n\"];\n22->10;\n23[label=\"Node Type: NEW VARIABLE 23\n\"];\n23->24;\n24[label=\"Node Type: EXPRESSION 24\n\"];\n24->8;\n25[label=\"Node Type: END INLINE ASM 25\n\"];\n25->26;\n26[label=\"Node Type: RETURN 26\n\"];\n}\n" + }, + "Initializable": { + "_initializableSlot()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n", + "initializer()": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n1->2;\n2[label=\"Node Type: INLINE ASM 2\n\"];\n2->3;\n3[label=\"Node Type: NEW VARIABLE 3\n\"];\n3->4;\n4[label=\"Node Type: EXPRESSION 4\n\"];\n4->5;\n5[label=\"Node Type: EXPRESSION 5\n\"];\n5->6;\n6[label=\"Node Type: IF 6\n\"];\n6->8[label=\"True\"];\n6->7[label=\"False\"];\n7[label=\"Node Type: END_IF 7\n\"];\n7->13;\n8[label=\"Node Type: IF 8\n\"];\n8->10[label=\"True\"];\n8->9[label=\"False\"];\n9[label=\"Node Type: END_IF 9\n\"];\n9->12;\n10[label=\"Node Type: EXPRESSION 10\n\"];\n10->11;\n11[label=\"Node Type: EXPRESSION 11\n\"];\n11->9;\n12[label=\"Node Type: EXPRESSION 12\n\"];\n12->7;\n13[label=\"Node Type: END INLINE ASM 13\n\"];\n13->14;\n14[label=\"Node Type: _ 14\n\"];\n14->15;\n15[label=\"Node Type: INLINE ASM 15\n\"];\n15->16;\n16[label=\"Node Type: IF 16\n\"];\n16->18[label=\"True\"];\n16->17[label=\"False\"];\n17[label=\"Node Type: END_IF 17\n\"];\n17->21;\n18[label=\"Node Type: EXPRESSION 18\n\"];\n18->19;\n19[label=\"Node Type: EXPRESSION 19\n\"];\n19->20;\n20[label=\"Node Type: EXPRESSION 20\n\"];\n20->17;\n21[label=\"Node Type: END INLINE ASM 21\n\"];\n}\n" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/yul-solady.sol b/tests/e2e/solc_parsing/test_data/yul-solady.sol new file mode 100644 index 0000000000..b82dddcf45 --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/yul-solady.sol @@ -0,0 +1,75 @@ +contract C { + // Snippet of the _checkpointPushDiff function that was making slither crashes + // https://github.com/Vectorized/solady/blob/9298d096feb87de9a8873a704ff98f6892064c65/src/tokens/ERC20Votes.sol#L339-L361 + function _checkpointPushDiff(uint256 lengthSlot, uint256 key, uint256 amount, bool isAdd) + private + returns(uint256 newValue) + { + /// @solidity memory-safe-assembly + assembly { + let lengthSlotPacked := sload(lengthSlot) + for { let n := shr(208, shl(160, lengthSlotPacked)) } 1 {} { + if iszero(n) { + if iszero(or(isAdd, iszero(amount))) { + mstore(0x00, 0x5915f686) // `ERC5805CheckpointValueUnderflow()`. + revert(0x1c, 0x04) + } + newValue := amount + if iszero(or(eq(newValue, address()), shr(160, newValue))) { + sstore(lengthSlot, or(or(key, shl(48, 1)), shl(96, newValue))) + break + } + sstore(lengthSlot, or(or(key, shl(48, 1)), shl(96, address()))) + sstore(not(lengthSlot), newValue) + break + } + let checkpointSlot := add(sub(n, 1), lengthSlot) + } + } + } +} + + +// Snippet of the Initializable contract that was making slither crashes +// https://github.com/Vectorized/solady/blob/9298d096feb87de9a8873a704ff98f6892064c65/src/utils/Initializable.sol#L7 +contract Initializable { + bytes32 private constant _INTIALIZED_EVENT_SIGNATURE = + 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2; + bytes32 private constant _INITIALIZABLE_SLOT = + 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffbf601132; + + + function _initializableSlot() internal pure virtual returns (bytes32) { + return _INITIALIZABLE_SLOT; + } + + modifier initializer() virtual { + bytes32 s = _initializableSlot(); + /// @solidity memory-safe-assembly + assembly { + let i := sload(s) + // Set `initializing` to 1, `initializedVersion` to 1. + sstore(s, 3) + // If `!(initializing == 0 && initializedVersion == 0)`. + if i { + // If `!(address(this).code.length == 0 && initializedVersion == 1)`. + if iszero(lt(extcodesize(address()), eq(shr(1, i), 1))) { + mstore(0x00, 0xf92ee8a9) // `InvalidInitialization()`. + revert(0x1c, 0x04) + } + s := shl(shl(255, i), s) // Skip initializing if `initializing == 1`. + } + } + _; + /// @solidity memory-safe-assembly + assembly { + if s { + // Set `initializing` to 0, `initializedVersion` to 1. + sstore(s, 2) + // Emit the {Initialized} event. + mstore(0x20, 1) + log1(0x20, 0x20, _INTIALIZED_EVENT_SIGNATURE) + } + } + } +} \ No newline at end of file diff --git a/tests/unit/slithir/test_data/assembly_sstore_sload.sol b/tests/unit/slithir/test_data/assembly_sstore_sload.sol new file mode 100644 index 0000000000..bf32d1f9da --- /dev/null +++ b/tests/unit/slithir/test_data/assembly_sstore_sload.sol @@ -0,0 +1,29 @@ +contract Test{ + + uint variable; + + function read() internal { + assembly { + let read_value := sload(variable.slot) + } + } + + function read_parameter(uint slot) internal { + assembly { + let read_value := sload(slot) + } + } + + function write() internal { + assembly { + sstore(variable.slot, 1) + } + } + + function write_parameter(uint slot) internal { + assembly { + sstore(slot, 1) + } + } + +} \ No newline at end of file diff --git a/tests/unit/slithir/test_yul_parser_assembly_slot.py b/tests/unit/slithir/test_yul_parser_assembly_slot.py index da800b55e0..a2500288fd 100644 --- a/tests/unit/slithir/test_yul_parser_assembly_slot.py +++ b/tests/unit/slithir/test_yul_parser_assembly_slot.py @@ -38,3 +38,29 @@ def test_yul_parser_assembly_slot(solc_binary_path) -> None: assert isinstance(value.value, StateVariable) elif value.value.name == "bucketId": assert isinstance(value.value, LocalVariable) + +def test_yul_parser_assembly_slot(solc_binary_path) -> None: + # mstore(0x0, bucketId) + # mstore(0x20, _counters.slot) + data = {"0x0": "bucketId", "0x20": "_counters"} + + solc_path = solc_binary_path("0.8.18") + slither = Slither(Path(TEST_DATA_DIR, "assembly_sstore_sload.sol").as_posix(), solc=solc_path) + + contract = slither.get_contract_from_name("Test")[0] + + read = contract.get_function_from_full_name("read()") + # Sload is converted to an assignement + assert len(read.all_solidity_calls()) == 0 + + read_parameter = contract.get_function_from_full_name("read_parameter(uint256)") + # Sload is kept because the slot is dynamic + assert len(read_parameter.all_solidity_calls()) == 1 + + write = contract.get_function_from_full_name("write()") + # Sstore is converted to an assignement + assert len(write.all_solidity_calls()) == 0 + + write_parameter = contract.get_function_from_full_name("write_parameter(uint256)") + # Sstore is kept because the slot is dynamic + assert len(write_parameter.all_solidity_calls()) == 1 From 78eefb116afd819d636bdb86c974ef3fc8f3a789 Mon Sep 17 00:00:00 2001 From: Josselin Feist Date: Thu, 20 Feb 2025 12:03:56 +0100 Subject: [PATCH 2/4] Update slither/visitors/slithir/expression_to_slithir.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- slither/visitors/slithir/expression_to_slithir.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 2a132339cb..67f591c9f8 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -406,7 +406,9 @@ def _post_call_expression(self, expression: CallExpression) -> None: self._result.append(var) set_val(expression, val) - elif (called.name == "sload(uint256)" or called.name == "sstore(uint256,uint256)") and (len(args)>0 and isinstance(args[0], StateVariable)): + elif (called.name == "sload(uint256)" or called.name == "sstore(uint256,uint256)") and ( + len(args) > 0 and isinstance(args[0], StateVariable) + ): # parse_yul._parse_yul_magic_suffixes does a best effort tentative to retrieve # the right state variable on .slot access # From 33b4bbaa394e773b958332dfa18e92a0e582af32 Mon Sep 17 00:00:00 2001 From: Josselin Feist Date: Thu, 20 Feb 2025 12:04:03 +0100 Subject: [PATCH 3/4] Update tests/unit/slithir/test_yul_parser_assembly_slot.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- tests/unit/slithir/test_yul_parser_assembly_slot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/slithir/test_yul_parser_assembly_slot.py b/tests/unit/slithir/test_yul_parser_assembly_slot.py index a2500288fd..56f6a5c6de 100644 --- a/tests/unit/slithir/test_yul_parser_assembly_slot.py +++ b/tests/unit/slithir/test_yul_parser_assembly_slot.py @@ -39,6 +39,7 @@ def test_yul_parser_assembly_slot(solc_binary_path) -> None: elif value.value.name == "bucketId": assert isinstance(value.value, LocalVariable) + def test_yul_parser_assembly_slot(solc_binary_path) -> None: # mstore(0x0, bucketId) # mstore(0x20, _counters.slot) From 116a31ac426ba422ec0b467afd2b26587df0b57f Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Thu, 20 Feb 2025 12:09:47 +0100 Subject: [PATCH 4/4] fix linters --- slither/visitors/slithir/expression_to_slithir.py | 6 ++++-- tests/unit/slithir/test_yul_parser_assembly_slot.py | 5 +---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 67f591c9f8..f4346f81dc 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -406,8 +406,10 @@ def _post_call_expression(self, expression: CallExpression) -> None: self._result.append(var) set_val(expression, val) - elif (called.name == "sload(uint256)" or called.name == "sstore(uint256,uint256)") and ( - len(args) > 0 and isinstance(args[0], StateVariable) + elif ( + called.name in ["sload(uint256)", "sstore(uint256,uint256)"] + and len(args) > 0 + and isinstance(args[0], StateVariable) ): # parse_yul._parse_yul_magic_suffixes does a best effort tentative to retrieve # the right state variable on .slot access diff --git a/tests/unit/slithir/test_yul_parser_assembly_slot.py b/tests/unit/slithir/test_yul_parser_assembly_slot.py index 56f6a5c6de..593b65a5dc 100644 --- a/tests/unit/slithir/test_yul_parser_assembly_slot.py +++ b/tests/unit/slithir/test_yul_parser_assembly_slot.py @@ -40,10 +40,7 @@ def test_yul_parser_assembly_slot(solc_binary_path) -> None: assert isinstance(value.value, LocalVariable) -def test_yul_parser_assembly_slot(solc_binary_path) -> None: - # mstore(0x0, bucketId) - # mstore(0x20, _counters.slot) - data = {"0x0": "bucketId", "0x20": "_counters"} +def test_yul_parser_sstore_sload(solc_binary_path) -> None: solc_path = solc_binary_path("0.8.18") slither = Slither(Path(TEST_DATA_DIR, "assembly_sstore_sload.sol").as_posix(), solc=solc_path)