[Bug-Candidate]: State Variable Writes in Library Calls Inside Functions Are Not Tracked

[bossjoker1]

Describe the issue:

I want to track the state variable writes in the function MinterRole._addMinter(address), but I found that it only correctly returns the state variable _minters that the function read.


Then I tried to insert the following code snippet in the function.py , and it was able to solve the problem, but I feel it's not elegant enough.

+        # consider state variables written in library calls
+        from slither.slithir.operations import LibraryCall
+        lbc_nodes = [x for x in self.nodes if x.library_calls]
+        for node in lbc_nodes:
+            for ir in node.irs:
+                if not isinstance(ir, LibraryCall):
+                    continue
+                for  (param, arg) in zip(ir.function.parameters, ir.arguments):
+                    if param in ir.function.variables_written and isinstance(arg, StateVariable):
+                        if arg not in self._state_vars_written:
+                            self._state_vars_written.append(arg)

Code example to reproduce the issue:

Library:

library Roles {
    struct Role {
        mapping (address => bool) bearer;
    }

    /**
     * @dev Give an account access to this role.
     */
    function add(Role storage role, address account) internal {
        require(!has(role, account), "Roles: account already has role");
        role.bearer[account] = true;
    }

    /**
     * @dev Remove an account's access to this role.
     */
    function remove(Role storage role, address account) internal {
        require(has(role, account), "Roles: account does not have role");
        role.bearer[account] = false;
    }

    /**
     * @dev Check if an account has this role.
     * @return bool
     */
    function has(Role storage role, address account) internal view returns (bool) {
        require(account != address(0), "Roles: account is the zero address");
        return role.bearer[account];
    }
}

Contract:

pragma solidity ^0.5.0;

contract Context {
    // Empty internal constructor, to prevent people from mistakenly deploying
    // an instance of this contract, which should be used via inheritance.
    constructor () internal { }
    // solhint-disable-previous-line no-empty-blocks

    function _msgSender() internal view returns (address payable) {
        return msg.sender;
    }

    function _msgData() internal view returns (bytes memory) {
        this; // silence state mutability warning without generating bytecode - see https://github.com/ethereum/solidity/issues/2691
        return msg.data;
    }
}


contract MinterRole is Context {
    using Roles for Roles.Role;

    event MinterAdded(address indexed account);
    event MinterRemoved(address indexed account);

    Roles.Role private _minters;

    constructor () internal {
        _addMinter(_msgSender());
    }

    modifier onlyMinter() {
        require(isMinter(_msgSender()), "MinterRole: caller does not have the Minter role");
        _;
    }

    function isMinter(address account) public view returns (bool) {
        return _minters.has(account);
    }

    function addMinter(address account) public onlyMinter {
        _addMinter(account);
    }

    function renounceMinter() public {
        _removeMinter(_msgSender());
    }

    function _addMinter(address account) internal {
        _minters.add(account);
        emit MinterAdded(account);
    }

    function _removeMinter(address account) internal {
        _minters.remove(account);
        emit MinterRemoved(account);
    }
}

Version:

0.10.4

Relevant log output:

No response