Nonce is not incremented after using signature for policy validation

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/PolicyValidator.sol#L54

Vulnerability details

Impact

Nonce is not incremented after using signature for policy validation

Proof of Concept

In PolicyValidator.sol

there is a function

    function isPolicySignatureValid(
        address account,
        address to,
        uint256 value,
        bytes memory data,
        Enum.Operation operation,
        bytes calldata signatures
    ) external view returns (bool) {
        // Get nonce from safe
        uint256 nonce = IGnosisSafe(account).nonce();

        // Build transaction struct hash
        bytes32 transactionStructHash = TypeHashHelper._buildTransactionStructHash(
            TypeHashHelper.Transaction({
                to: to,
                value: value,
                data: data,
                operation: uint8(operation),
                account: account,
                executor: address(0),
                nonce: nonce
            })
        );

        // Validate signature
        return isPolicySignatureValid(account, transactionStructHash, signatures);
    }

the code query the nonce from the safe wallet

but after consume the nonce to validate the policy signature, the nonce is not incremented

this allows signature replay and the same signature can be used multiple to bypass the isPolicyValidation check until the expire expires or until the nonce is incremented on the safe wallet side

Tools Used

manual review

Recommended Mitigation Steps

increment the nonce to not code reuse the nonce and signature when validating the policy signature validation

Assessed type

Access Control

[0xad1onchain]

Invalid, safe internally increments the nonce

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #129

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

As the Sponsor has correctly specified, the non-mutating PolicyValidator:: isPolicySignatureValid function is meant to be invoked as part of a Gnosis Safe transaction execution flow. In such a flow, the nonce would have been incremented before it has been validated thus ensuring that policy validations are atomic.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid