sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

BAICE - Missing return value in`SafeGuard:checkTransaction`, cannot check whether this transaction has been Vetoed #174

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

BAICE

medium

Missing return value inSafeGuard:checkTransaction, cannot check whether this transaction has been Vetoed

Summary

No return value in SafeGuard:checkTransaction, cannot check whether this transaction has been vetoed.

Vulnerability Detail

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/zodiac/core/SafeGuard.sol#L48-L75

This checkTransaction method check whether this transactionHash list (calculated by transaction params ) has been vetoed, no return value .

Impact

This function of checkTransaction will not work , and will influence other function results based on this checkTransaction method .

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/zodiac/core/SafeGuard.sol#L48-L75

function checkTransaction(
        address to,
        uint256 value,
        bytes memory data,
        Enum.Operation operation,
        uint256,
        uint256,
        uint256,
        address,
        address payable,
        bytes memory,
        address
    ) external view override {
        // cycles through possible transactions
        for (uint256 i = 0; i < nonces.length; i++) {
            bytes32 transactionHash = IReality(_msgSender()).getTransactionHash(
                to,
                value,
                data,
                operation,
                nonces[i]
            );
            require(
                !transactionHashes[transactionHash],
                "SafeGuard: transaction has been vetoed"
            );
        }
    }

Tool used

Manual Review VSCode

Recommendation

Add boolean return value

function checkTransaction(
        address to,
        uint256 value,
        bytes memory data,
        Enum.Operation operation,
        uint256,
        uint256,
        uint256,
        address,
        address payable,
        bytes memory,
        address
@-    ) external view override  {
@+    ) external view override returns (bool) {
    ---
return true ;
sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { since its not mention its the intended behavior i believe}

nevillehuang commented 6 months ago

Invalid, check done here, explicit return value is not required