Merge branch 'dev' of github.com:crytic/slither into dev

setup.py

@@ -14,10 +14,10 @@
  install_requires=[
  "prettytable>=0.7.2",
  "pysha3>=1.0.2",
- "crytic-compile>=0.2.3",
- # "crytic-compile",
+ # "crytic-compile>=0.2.3",
+ "crytic-compile",
  ],
- # dependency_links=["git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile"],
+ dependency_links=["git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile"],
  license="AGPL-3.0",
  long_description=long_description,
  entry_points={

slither/detectors/all_detectors.py

@@ -6,7 +6,9 @@
 from .attributes.constant_pragma import ConstantPragma
 from .attributes.incorrect_solc import IncorrectSolc
 from .attributes.locked_ether import LockedEther
-from .functions.arbitrary_send import ArbitrarySend
+from .functions.arbitrary_send_eth import ArbitrarySendEth
+from .erc.erc20.arbitrary_send_erc20_no_permit import ArbitrarySendErc20NoPermit
+from .erc.erc20.arbitrary_send_erc20_permit import ArbitrarySendErc20Permit
 from .functions.suicidal import Suicidal
 
 # from .functions.complex_function import ComplexFunction
@@ -34,7 +36,7 @@
 from .operations.block_timestamp import Timestamp
 from .statements.calls_in_loop import MultipleCallsInLoop
 from .statements.incorrect_strict_equality import IncorrectStrictEquality
-from .erc.incorrect_erc20_interface import IncorrectERC20InterfaceDetection
+from .erc.erc20.incorrect_erc20_interface import IncorrectERC20InterfaceDetection
 from .erc.incorrect_erc721_interface import IncorrectERC721InterfaceDetection
 from .erc.unindexed_event_parameters import UnindexedERC20EventParameters
 from .statements.deprecated_calls import DeprecatedStandards

slither/detectors/erc/erc20/arbitrary_send_erc20.py

@@ -0,0 +1,95 @@
+from typing import List
+from slither.core.cfg.node import Node
+from slither.core.declarations.solidity_variables import SolidityVariable
+from slither.slithir.operations import HighLevelCall, LibraryCall
+from slither.core.declarations import Contract, Function, SolidityVariableComposed
+from slither.analyses.data_dependency.data_dependency import is_dependent
+from slither.core.compilation_unit import SlitherCompilationUnit
+
+
+class ArbitrarySendErc20:
+ """Detects instances where ERC20 can be sent from an arbitrary from address."""
+
+ def __init__(self, compilation_unit: SlitherCompilationUnit):
+ self._compilation_unit = compilation_unit
+ self._no_permit_results: List[Node] = []
+ self._permit_results: List[Node] = []
+
+ @property
+ def compilation_unit(self) -> SlitherCompilationUnit:
+ return self._compilation_unit
+
+ @property
+ def no_permit_results(self) -> List[Node]:
+ return self._no_permit_results
+
+ @property
+ def permit_results(self) -> List[Node]:
+ return self._permit_results
+
+ def _detect_arbitrary_from(self, contract: Contract):
+ for f in contract.functions:
+ all_high_level_calls = [
+ f_called[1].solidity_signature
+ for f_called in f.high_level_calls
+ if isinstance(f_called[1], Function)
+ ]
+ all_library_calls = [f_called[1].solidity_signature for f_called in f.library_calls]
+ if (
+ "transferFrom(address,address,uint256)" in all_high_level_calls
+ or "safeTransferFrom(address,address,address,uint256)" in all_library_calls
+ ):
+ if (
+ "permit(address,address,uint256,uint256,uint8,bytes32,bytes32)"
+ in all_high_level_calls
+ ):
+ ArbitrarySendErc20._arbitrary_from(f.nodes, self._permit_results)
+ else:
+ ArbitrarySendErc20._arbitrary_from(f.nodes, self._no_permit_results)
+
+ @staticmethod
+ def _arbitrary_from(nodes: List[Node], results: List[Node]):
+ """Finds instances of (safe)transferFrom that do not use msg.sender or address(this) as from parameter."""
+ for node in nodes:
+ for ir in node.irs:
+ if (
+ isinstance(ir, HighLevelCall)
+ and isinstance(ir.function, Function)
+ and ir.function.solidity_signature == "transferFrom(address,address,uint256)"
+ and not (
+ is_dependent(
+ ir.arguments[0],
+ SolidityVariableComposed("msg.sender"),
+ node.function.contract,
+ )
+ or is_dependent(
+ ir.arguments[0],
+ SolidityVariable("this"),
+ node.function.contract,
+ )
+ )
+ ):
+ results.append(ir.node)
+ elif (
+ isinstance(ir, LibraryCall)
+ and ir.function.solidity_signature
+ == "safeTransferFrom(address,address,address,uint256)"
+ and not (
+ is_dependent(
+ ir.arguments[1],
+ SolidityVariableComposed("msg.sender"),
+ node.function.contract,
+ )
+ or is_dependent(
+ ir.arguments[1],
+ SolidityVariable("this"),
+ node.function.contract,
+ )
+ )
+ ):
+ results.append(ir.node)
+
+ def detect(self):
+ """Detect transfers that use arbitrary `from` parameter."""
+ for c in self.compilation_unit.contracts_derived:
+ self._detect_arbitrary_from(c)

slither/detectors/erc/erc20/arbitrary_send_erc20_no_permit.py

@@ -0,0 +1,45 @@
+from typing import List
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.utils.output import Output
+from .arbitrary_send_erc20 import ArbitrarySendErc20
+
+
+class ArbitrarySendErc20NoPermit(AbstractDetector):
+ """
+ Detect when `msg.sender` is not used as `from` in transferFrom
+ """
+
+ ARGUMENT = "arbitrary-send-erc20"
+ HELP = "transferFrom uses arbitrary `from`"
+ IMPACT = DetectorClassification.HIGH
+ CONFIDENCE = DetectorClassification.HIGH
+
+ WIKI = "https://github.com/trailofbits/slither/wiki/Detector-Documentation#arbitrary-send-erc20"
+
+ WIKI_TITLE = "Arbitrary `from` in transferFrom"
+ WIKI_DESCRIPTION = "Detect when `msg.sender` is not used as `from` in transferFrom."
+ WIKI_EXPLOIT_SCENARIO = """
+```solidity
+ function a(address from, address to, uint256 amount) public {
+ erc20.transferFrom(from, to, am);
+ }
+```
+Alice approves this contract to spend her ERC20 tokens. Bob can call `a` and specify Alice's address as the `from` parameter in `transferFrom`, allowing him to transfer Alice's tokens to himself."""
+
+ WIKI_RECOMMENDATION = """
+Use `msg.sender` as `from` in transferFrom.
+"""
+
+ def _detect(self) -> List[Output]:
+ """"""
+ results: List[Output] = []
+
+ arbitrary_sends = ArbitrarySendErc20(self.compilation_unit)
+ arbitrary_sends.detect()
+ for node in arbitrary_sends.no_permit_results:
+ func = node.function
+ info = [func, " uses arbitrary from in transferFrom: ", node, "\n"]
+ res = self.generate_result(info)
+ results.append(res)
+
+ return results

slither/detectors/erc/erc20/arbitrary_send_erc20_permit.py

@@ -0,0 +1,53 @@
+from typing import List
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.utils.output import Output
+from .arbitrary_send_erc20 import ArbitrarySendErc20
+
+
+class ArbitrarySendErc20Permit(AbstractDetector):
+ """
+ Detect when `msg.sender` is not used as `from` in transferFrom along with the use of permit.
+ """
+
+ ARGUMENT = "arbitrary-send-erc20-permit"
+ HELP = "transferFrom uses arbitrary from with permit"
+ IMPACT = DetectorClassification.HIGH
+ CONFIDENCE = DetectorClassification.MEDIUM
+
+ WIKI = "https://github.com/trailofbits/slither/wiki/Detector-Documentation#arbitrary-send-erc20-permit"
+
+ WIKI_TITLE = "Arbitrary `from` in transferFrom used with permit"
+ WIKI_DESCRIPTION = (
+ "Detect when `msg.sender` is not used as `from` in transferFrom and permit is used."
+ )
+ WIKI_EXPLOIT_SCENARIO = """
+```solidity
+ function bad(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) public {
+ erc20.permit(from, address(this), value, deadline, v, r, s);
+ erc20.transferFrom(from, to, value);
+ }
+```
+If an ERC20 token does not implement permit and has a fallback function e.g. WETH, transferFrom allows an attacker to transfer all tokens approved for this contract."""
+
+ WIKI_RECOMMENDATION = """
+Ensure that the underlying ERC20 token correctly implements a permit function.
+"""
+
+ def _detect(self) -> List[Output]:
+ """"""
+ results: List[Output] = []
+
+ arbitrary_sends = ArbitrarySendErc20(self.compilation_unit)
+ arbitrary_sends.detect()
+ for node in arbitrary_sends.permit_results:
+ func = node.function
+ info = [
+ func,
+ " uses arbitrary from in transferFrom in combination with permit: ",
+ node,
+ "\n",
+ ]
+ res = self.generate_result(info)
+ results.append(res)
+
+ return results

slither/detectors/functions/arbitrary_send.py → slither/detectors/functions/arbitrary_send_eth.py

@@ -90,8 +90,8 @@ def detect_arbitrary_send(contract: Contract):
  return ret
 
 
-class ArbitrarySend(AbstractDetector):
- ARGUMENT = "arbitrary-send"
+class ArbitrarySendEth(AbstractDetector):
+ ARGUMENT = "arbitrary-send-eth"
  HELP = "Functions that send Ether to arbitrary destinations"
  IMPACT = DetectorClassification.HIGH
  CONFIDENCE = DetectorClassification.MEDIUM
@@ -104,7 +104,7 @@ class ArbitrarySend(AbstractDetector):
  # region wiki_exploit_scenario
  WIKI_EXPLOIT_SCENARIO = """
 ```solidity
-contract ArbitrarySend{
+contract ArbitrarySendEth{
  address destination;
  function setDestination(){
  destination = msg.sender;

slither/printers/summary/variable_order.py

@@ -28,7 +28,7 @@ def output(self, _filename):
  txt += f"\n{contract.name}:\n"
  table = MyPrettyTable(["Name", "Type", "Slot", "Offset"])
  for variable in contract.state_variables_ordered:
- if not variable.is_constant:
+ if not variable.is_constant and not variable.is_immutable:
  slot, offset = contract.compilation_unit.storage_layout_of(contract, variable)
  table.add_row([variable.canonical_name, str(variable.type), slot, offset])
 

slither/tools/read_storage/README.md

@@ -46,13 +46,13 @@ slither-read-storage 0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8 --layout --rpc-u
 To view only the slot of the `slot0` structure variable, pass `--variable-name slot0`:
 
 ```shell
-slither-read-storage 0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8 --variable-name slot0 --rpc-url https://mainnet.infura.io/v3/04942f7970ef41cc847a147bc64e460e --value
+slither-read-storage 0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8 --variable-name slot0 --rpc-url $RPC_URL --value
 ```
 
 To view a member of the `slot0` struct, pass `--struct-var tick`
 
 ```shell
-slither-read-storage 0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8 --variable-name slot0 --rpc-url https://mainnet.infura.io/v3/04942f7970ef41cc847a147bc64e460e --value --struct-var tick
+slither-read-storage 0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8 --variable-name slot0 --rpc-url $RPC_URL --value --struct-var tick
 ```
 
 Retrieve the ERC20 balance slot of an account:

tests/detectors/arbitrary-send-erc20-permit/0.4.25/arbitrary_send_erc20_permit.sol

@@ -0,0 +1,57 @@
+pragma solidity 0.4.25;
+
+library SafeERC20 {
+ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal {}
+}
+
+interface IERC20 {
+ function transferFrom(address, address, uint256) external returns(bool);
+ function permit(address, address, uint256, uint256, uint8, bytes32, bytes32) external;
+}
+
+contract ERC20 is IERC20 {
+ function transferFrom(address from, address to, uint256 amount) external returns(bool) {
+ return true;
+ }
+ function permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s) external {}
+}
+
+contract C {
+ using SafeERC20 for IERC20;
+
+ IERC20 erc20;
+ address notsend;
+ address send;
+
+ constructor() public {
+ erc20 = new ERC20();
+ notsend = address(0x3);
+ send = msg.sender;
+ }
+
+ function bad1(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) public {
+ erc20.permit(from, address(this), value, deadline, v, r, s);
+ erc20.transferFrom(from, to, value);
+ }
+
+ // This is not detected
+ function bad2(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) public {
+ int_transferFrom(from,value, deadline, v, r, s, to);
+ }
+
+ function int_transferFrom(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) internal {
+ erc20.permit(from, address(this), value, deadline, v, r, s);
+ erc20.transferFrom(from, to, value);
+ }
+
+ function bad3(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) external {
+ erc20.permit(from, address(this), value, deadline, v, r, s);
+ erc20.safeTransferFrom(from, to, value);
+ } 
+
+ function bad4(address from, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s, address to) external {
+ erc20.permit(from, address(this), value, deadline, v, r, s);
+ SafeERC20.safeTransferFrom(erc20, from, to, value);
+ }
+
+}