crytic / slither

Static Analyzer for Solidity and Vyper
https://blog.trailofbits.com/2018/10/19/slither-a-solidity-static-analysis-framework/
GNU Affero General Public License v3.0
5.24k stars 956 forks source link

how to read caller function parameters #2533

Open alexanderhawl opened 4 weeks ago

alexanderhawl commented 4 weeks ago

Describe the issue:

There is a smart contract

// SPDX-License-Identifier: BSL-1.1

pragma solidity ^0.8.26;

library TransferHelper {
    /// @notice Transfers tokens from the targeted address to the given destination
    /// @notice Errors with 'STF' if transfer fails
    /// @param token The contract address of the token to be transferred
    /// @param from The originating address from which the tokens will be transferred
    /// @param to The destination address of the transfer
    /// @param value The amount to be transferred
    function safeTransferFrom(
        address token,
        address from,
        address to,
        uint256 value
    ) internal {
        (bool success, bytes memory data) =
            token.call(abi.encodeWithSelector(IERC20.transferFrom.selector, from, to, value));
        require(success && (data.length == 0 || abi.decode(data, (bool))), 'STF');
    }

    /// @notice Transfers tokens from msg.sender to a recipient
    /// @dev Errors with ST if transfer fails
    /// @param token The contract address of the token which will be transferred
    /// @param to The recipient of the transfer
    /// @param value The value of the transfer
    function safeTransfer(
        address token,
        address to,
        uint256 value
    ) internal {
        (bool success, bytes memory data) = token.call(abi.encodeWithSelector(IERC20.transfer.selector, to, value));
        require(success && (data.length == 0 || abi.decode(data, (bool))), 'ST');
    }
}

contract TokenFee{
    address public ETH=0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

    function transferFee(
        uint256 amount,
        address token,
        address from,
        address to
    ) public returns (uint256) {
        uint256 fee = 10000;
        if (fee > 0) {
            if (token != ETH) {
                // ERC20 token
                if (from == address(this)) {
                    TransferHelper.safeTransfer(token, to, fee);
                } else {
                    // safeTransferFrom requires approval
                    TransferHelper.safeTransferFrom(token, from, to, fee);
                }
            } else {
                require(
                    from == address(this),
                    "can only transfer eth from the router address"
                );

                // Native ether
                (bool success, ) = to.call{value: fee}("");
                require(success, "transfer failed in transferFee");
            }
            return fee;
        } else {
            return 0;
        }
    }
}

I use slither in python, to detect the safeTransferFrom whether limit the from parameter. This is my code

from slither.slither import Slither  
file='TokenFee'
ctsl=Slither(file,solc_args="--optimize --optimize-runs 200")
ct=ctsl.contracts
for contract in ct: 
        funcs=contract.functions
        for func in funcs:
            fps=[]
            for fp in func.parameters:
                fps.append(fp.name)
            for scs in func.all_high_level_calls():
                if str(scs[1])=='safeTransferFrom':

                    frm=str(scs[1].parameters[1])
                    t=str(scs[1].parameters[2])
                    if (frm in fps) and (t in fps):
                        print('vuln')                         

But I found scs[1].parameters are parameters 'token' 'from','to','value' which are in TransferHelper smart contract's safeTransferFrom, not 'token','from','to','fee' in TokenFee contract's safeTransferFrom. How to get the right parameter 'token','from','to','fee'?

Code example to reproduce the issue:

from slither.slither import Slither  
file='TokenFee'
ctsl=Slither(file,solc_args="--optimize --optimize-runs 200")
ct=ctsl.contracts
for contract in ct: 
        funcs=contract.functions
        for func in funcs:
            fps=[]
            for fp in func.parameters:
                fps.append(fp.name)
            for scs in func.all_high_level_calls():
                if str(scs[1])=='safeTransferFrom':

                    frm=str(scs[1].parameters[1])
                    t=str(scs[1].parameters[2])
                    if (frm in fps) and (t in fps):
                        print('vuln')                         

Version:

0.10.3

Relevant log output:

No response

0xalpharush commented 4 weeks ago

You are looking for arguments of the call https://crytic.github.io/slither/slither/slithir/operations/call.html#Call.arguments. The parameters are what is in the function defintion.

Here's an example of how you'd do this https://github.com/crytic/slither/blob/9a5b4f5cff9db6475c5badff18f9882180d5f344/slither/detectors/erc/erc20/arbitrary_send_erc20.py#L54-L64

For your example here's roughly how that translates

for contract in ct: 
        funcs=contract.functions
        for func in funcs:
            fps=[]
            for fp in func.parameters:
                fps.append(fp.name)
            for block in func.nodes:
                for operation in block.irs:
                    if isinstance(operation, HighLevelCall) 
                    and isinstance(operation.function, Function) 
                    and operation.function.name == "safeTransferFrom": # use sig or check length of args to prevent IndexError's

                        frm=str(scs[1].arguments[1])
                        t=str(scs[1].arguments[2])

                        if (frm in fps) and (t in fps):
                            print('vuln')