Invalid handling of try-catch code

[pdyraga]

$ slither --version
0.8.0

Please consider the following code:

        try
            self.dkgValidator.validate(result, self.seed, self.startBlock)
        returns (bool isValid, string memory errorMsg) {
            if (isValid) {
                revert("unjustified challenge");
            }

            emit DkgResultChallenged(
                self.submittedResultHash,
                msg.sender,
                errorMsg
            );
        } catch {
            // if the validation reverted we consider the DKG result as invalid
            emit DkgResultChallenged(
                self.submittedResultHash,
                msg.sender,
                "validation reverted"
            );
        }

Slither says:

INFO:Detectors:
DKG.challengeResult(DKG.Data,DKG.Result).isValid (contracts/libraries/DKG.sol#408) is a local variable never initialized
DKG.challengeResult(DKG.Data,DKG.Result).errorMsg (contracts/libraries/DKG.sol#408) is a local variable never initialized
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables
INFO:Detectors:
DKG.challengeResult(DKG.Data,DKG.Result) (contracts/libraries/DKG.sol#383-441) ignores return value by self.dkgValidator.validate(result,self.seed,self.startBlock) (contracts/libraries/DKG.sol#404-425)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return
INFO:Detectors:
Variable 'DKG.challengeResult(DKG.Data,DKG.Result).isValid (contracts/libraries/DKG.sol#408)' in DKG.challengeResult(DKG.Data,DKG.Result) (contracts/libraries/DKG.sol#383-441) potentially used before declaration: isValid (contracts/libraries/DKG.sol#409)
Variable 'DKG.challengeResult(DKG.Data,DKG.Result).errorMsg (contracts/libraries/DKG.sol#408)' in DKG.challengeResult(DKG.Data,DKG.Result) (contracts/libraries/DKG.sol#383-441) potentially used before declaration: DkgResultChallenged(self.submittedResultHash,msg.sender,errorMsg) (contracts/libraries/DKG.sol#413-417)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#pre-declaration-usage-of-local-variables

This is a pretty standard try-catch block from Solidity.

[frangio]

Ran into this as well. The try expression triggers an "unused return value" warning, even though it is assigned to a variable and used. Another reproduction:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.2;

interface Foo {
    function foo() external returns (uint);
}

contract Test {
    function test(Foo a) external returns (uint y) {
        try a.foo() returns (uint x) {
            y = x;
        } catch {}
    }
}

C.test(IC).x (contracts/test.sol#10) is a local variable never initialized
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables

C.test(IC) (contracts/test.sol#9-13) ignores return value by a.foo() (contracts/test.sol#10-12)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unused-return

[ryanc414]

Got this same issue from using the Openzeppelin ERC721 library contract. It seems odd that slither is reporting errors in my dependencies - is this intentional? Is there any way to configure it to only check the primary sources and not deps?

[pbuda]

Got this same issue from using the Openzeppelin ERC721 library contract. It seems odd that slither is reporting errors in my dependencies - is this intentional? Is there any way to configure it to only check the primary sources and not deps?

You can use the filter_paths parameter to filter out openzeppelin contracts. I use json config file, and it's as simple as adding "filter_paths": "openzeppelin" to it.

[ryanc414]

Got this same issue from using the Openzeppelin ERC721 library contract. It seems odd that slither is reporting errors in my dependencies - is this intentional? Is there any way to configure it to only check the primary sources and not deps?

You can use the filter_paths parameter to filter out openzeppelin contracts. I use json config file, and it's as simple as adding "filter_paths": "openzeppelin" to it.

Yep, I figured that out thanks. Seems like a strange design choice to include dependencies by default though.

[thedavidmeister]

seems related or same as https://github.com/crytic/slither/issues/511

[QEDK]

Bumping this, we're facing the same issue 👍