sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

ravikiran.web3 - PassThroughWalletImpl's setPassThrough() function can accept address(0x0) could cause loss of funds #7

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ravikiran.web3

medium

PassThroughWalletImpl's setPassThrough() function can accept address(0x0) could cause loss of funds

Summary

PassThroughWalletImpl contract has a function to manage the passthrough address, managable by the owner. The set function can be called by owner. The set function does not validate of address(0x0).

Since this is managed by the owner and can be corrected, it is marked as medium

Vulnerability Detail

If the owner by mistake sets the passthrough address to address(0x0). Although he has the ability to recover by setting it back to the correct value. But the risk is around the passThroughTokens() function where tokens are routed via this function, will be transfer from Wallet to address(0x0) and will be permanently lost.

Impact

Loss of funds as they could be routed to address(0x0)

Code Snippet

https://github.com/0xSplits/splits-pass-through-wallet/blob/203badc970b9bb2216cf2ae0e93dcb0a0de19151/src/PassThroughWalletImpl.sol#L128

function passThroughTokens(address[] calldata tokens_) external pausable returns (uint256[] memory amounts) { address passThrough = $passThrough; uint256 length = tokens.length; amounts = new uint256; for (uint256 i; i < length;) { address token = tokens_[i]; uint256 amount = token._balanceOf(address(this)); amounts[i] = amount; token._safeTransfer(_passThrough, amount);

        unchecked {
            ++i;
        }
    }

    emit PassThrough(tokens_, amounts);
}

Tool used

Manual Review

Recommendation

Check for the address passed to setPassThrough() to be a valid not, specifically, not address(0x0)