False positive on contracts-that-lock-ether

[mrice32]

The ether locking detector doesn't seem to be able to follow library calls, so if the withdraw is implemented via a delegatecall to a library, the detector fires despite the contract implementing a perfectly valid withdraw function. Here's a sample contract that illustrates the problem:

/*
  Test
*/
pragma solidity ^0.5.0;

library Lib {
    struct State {
        uint balance;
    }

    function _deposit(State storage state) external {
        state.balance += msg.value;
    }

    function _withdraw(State storage state, uint value) external {
        require(value <= state.balance);
        state.balance -= value;
        msg.sender.transfer(value);
    }
}

contract Test {
    using Lib for Lib.State;

    Lib.State public state;

    function deposit() external payable {
        state._deposit();
    }

    function withdraw(uint value) external {
        state._withdraw(value);
    }
}

The following error message is generated by slither:

INFO:Detectors:
Contract locking ether found in OracleMock.sol:
        Contract Test has payable functions:
         - deposit (OracleMock.sol#27-29)
        But does not have a function to withdraw the ether
Reference: https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#contracts-that-lock-ether

cc @ptare

[montyly]

Hi @mrice32 , that's a good catch, thanks!