[Bug]: Unchecked blocks are identified as checked

[0xalpharush]

Describe the issue:

The is_checked attribute does not return false for functions that have unchecked blocks

Code example to reproduce the issue:

contract Test {

    function withdraw(address payable to, uint256 amount) public {
        unchecked {
            uint a = amount - amount;
        }
    }
}

Version:

0.8.3

Relevant log output:

Contract Test
        Function Test.withdraw(address,uint256) (*)
                Expression: a = amount - amount
                IRs:
                        TMP_0(uint256) = amount (c)- amount
                        a(uint256) := TMP_0(uint256)

[montyly]

is_checked is an attribute of the function, not the node or the IR, so it makes sense for the function to only consider the compiler version.

However, node have their own scope: https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/core/cfg/scope.py#L9 And is_checked is set to False in this scope for unchecked block: https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/solc_parsing/declarations/function.py#L1048 Which is used by the IR: https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/slithir/operations/binary.py#L180-L181

So this should definitely not return a checked operation, I am not sure why it happens

[smonicas]

The issue is that _parse_unchecked_block calls self._parse_statement with the new scope which is unchecked. https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/solc_parsing/declarations/function.py#L1048-L1051 However when parsing a VariableDeclarationStatement the new scope is not taken in consideration and it uses the node.underlying_node.scope which is still the old scope. https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/solc_parsing/declarations/function.py#L986-L987 https://github.com/crytic/slither/blob/8344524cd35a1edf37c1cc791eb08c8ffff645a8/slither/solc_parsing/declarations/function.py#L709-L714 The same is true when parsing other statements. In the following example the operations after the unchecked block will also be unchecked.

pragma solidity 0.8.3;

contract T {
  function withdraw(address payable to, uint256 amount) public {
   uint a;
   uint counter;
   unchecked {
       uint b = amount - amount;
       a = amount + b;
   }
   uint c = 5 + 7;
   for(uint i = 0; i < 10; i++) {counter++;}
  }
}

Contract T
	Function T.withdraw(address,uint256) (*)
		Expression: b = amount - amount
		IRs:
			TMP_0(uint256) = amount (c)- amount
			b(uint256) := TMP_0(uint256)
		Expression: a = amount + b
		IRs:
			TMP_1(uint256) = amount + b
			a(uint256) := TMP_1(uint256)
		Expression: c = 5 + 7
		IRs:
			TMP_2(uint256) = 5 + 7
			c(uint256) := TMP_2(uint256)
		Expression: i = 0
		IRs:
			i(uint256) := 0(uint256)
		Expression: i < 10
		IRs:
			TMP_3(bool) = i < 10
			CONDITION TMP_3
		Expression: counter ++
		IRs:
			TMP_4(uint256) := counter(uint256)
			counter(uint256) = counter + 1
		Expression: i ++
		IRs:
			TMP_5(uint256) := i(uint256)
			i(uint256) = i + 1

[alecmaly]

Thanks for identifying the functions.py file! I've spent some time updating it; I probably broke a bunch of stuff so I'm posting it here for others to validate and use it as a starting point.

I basically just added a scope property to a few of the _parse... functions. It's very possible other functions like _parse_cfg could be updated as well, but I don't have the bandwidth to focus on this and do thorough testing. There is probably a better way to do this but this seemed to work and get me where I need to be, hopefully it helps someone else as a temporary workaround. :)

Modified: function.py: https://github.com/alecmaly/slither/blob/4d35f7dd85ef2cf65c3f68f7af74f81292ae91c9/slither/solc_parsing/declarations/function.py

Modified from: https://github.com/crytic/slither/blob/635649207d52feff049d92cd75ec5b19edbfc61e/slither/solc_parsing/declarations/function.py

And here is a custom detector to test with. The goal is to identify complex unchecked {} blocks for closer inspection.

CDComplexUncheckedBlock.py

from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification

class CDComplexUncheckedBlock(AbstractDetector):  # pylint: disable=too-few-public-methods
    """
    Documentation
    """

    ARGUMENT = "CD-ComplexUncheckedBlock"  # slither will launch the detector with slither.py --mydetector
    HELP = "Help printed by slither"
    IMPACT = DetectorClassification.INFORMATIONAL
    CONFIDENCE = DetectorClassification.MEDIUM

    WIKI = "x"

    WIKI_TITLE = "Complex unchecked blocks"
    WIKI_DESCRIPTION = "validate logic + fuzz w/ Echidna?"
    WIKI_EXPLOIT_SCENARIO = "x"
    WIKI_RECOMMENDATION = "x"

    def _detect(self):
        """
        Complex Unchecked Block
        """


        unchecked_map = {}
        results = []
        unchecked_counter = 0
        for contract in self.compilation_unit.contracts:
            for function in contract.functions:
                for node in function.nodes:

                    # print to console to validate node expressions + their scopes
                    print(f"{node.scope.is_checked} - {str(node)}")

                    if not node.scope.is_checked:
                        if unchecked_counter == 0:
                            origin_node = node

                        unchecked_counter += 1
                        unchecked_map[origin_node] = unchecked_counter
                    else:
                        unchecked_counter = 0

        for node in unchecked_map:       
            info = [f"unchecked block w/ {unchecked_map[node]} nodes starting in func @ ", node]
            res = self.generate_result(info)
            results.append(res)

        return results