Add support for enhanced analyses through code comments

[montyly]

This is a WIP that enables the support for code comments to enhance Slither's capacities.

For now, two types of comments are accepted:

• security:non-reentrant - variables marked with this comment will not trigger reentrancy warnings
• security:write-protection="func_signature" - variables marked with this comment will require that any function that writes them have also an internal call to func_signature (function or modifier)

The long-term goal is to allow developers to provide simple information that can be used during a code review, and for tools.

For example on the following code

contract ReentrancyAndWrite{

    /// @custom:security non-reentrant
    /// @custom:security write-protection="onlyOwner()"
    I external_contract;

    modifier onlyOwner(){
        // lets assume there is an access control
        _;
    }   

    mapping(address => uint) balances;

    function withdraw() public{
        uint balance = balances[msg.sender];

        external_contract.external_call();

        balances[msg.sender] = 0;
        payable(msg.sender).transfer(balance);
    }
    
    function set_protected() public onlyOwner(){
        external_contract = I(msg.sender);
    }  

    function set_not_protected() public{
        external_contract = I(msg.sender);
    }
}

Here slither will:

• Not report the reentrancy in withdraw, as the variable is marked as non-reentrant
• Detect that set_not_protected writes the external_contract variable, while this variable should always be protected by a call to onlyOwner (sec:write-protection="onlyOwner()")

This is still a WIP, and ideas/feedback are welcome, so don't hesitate to share If you have ideas for other comments.

[lgtm-com]

This pull request introduces 1 alert when merging dbb5121 into e3392dd - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request introduces 1 alert when merging 6a262a0 into dc0cec2 - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request introduces 1 alert when merging a033259 into 8344524 - view on LGTM.com

new alerts:

• 1 for Unused import

[montyly]

I changed the comment to be in the form of @custom:security - this will allow splitting easily the comment into multiple lines, and might be better for the long term

[lgtm-com]

This pull request introduces 1 alert when merging d286f51 into 61bcec4 - view on LGTM.com

new alerts:

• 1 for Unused import

[zarifpour]

@montyly would it make sense to additionally support comments in the format (or similar to):

/// @custom:security ignore-<detector_filter>

Example:

/// @dev `data` is uninitialized to save gas
/// @custom:security ignore-uninitialized-local
function sample() {
    bytes memory data;
    for (uint256 j = 0; j < attributes.length - 1; j++) {
        if (!(attributes[j].length == 0)) {
            data = abi.encodePacked(data, attributes[j]);
        }
    }
}

[montyly]

@zarifpour yes, it's a good idea

We actually support //slither-disable-next-line(https://github.com/crytic/slither/wiki/Usage#triage-mode), but it would make sense in the long term to support it through @custom:security

One caveat through, this is not possible with older versions of solidity