code-423n4 / 2023-10-brahma-findings

8 stars 7 forks source link

getModulesPaginated does not return the correct data #395

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/ConsoleFallbackHandler.sol#L91-L96

Vulnerability details

Impact

In ConsoleFallbackHandler, you can call getModules() to return the first 10 modules:

function getModules() external view returns (address[] memory) {
        GnosisSafe safe = GnosisSafe(payable(msg.sender));
        (address[] memory array,) = safe.getModulesPaginated(SENTINEL_MODULES, 10);
        return array;
    }

However, there is a bug in the external call safe.getModulesPaginated. In the version that Brahma is using of Safe contracts, version 1.3.0, the function GnosisSafe.getModulesPaginated returns the wrong next pointer, leading to wrong data being returned.

This has been fixed in the newer versions of Safe contracts. Brahma still uses the old version 1.3.0

Tools Used

Manual Review

Recommended Mitigation Steps

Upgrade to a more recent version of Safe.

Assessed type

Context

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #16

c4-judge commented 10 months ago

alex-ppg marked the issue as not a duplicate

c4-sponsor commented 10 months ago

0xad1onchain (sponsor) acknowledged

alex-ppg commented 10 months ago

As the relevant PR of the Gnosis Safe repository details, the misbehaviour arises:

The relevant code of Brahma does not actually use the next pointer, meaning that it is insecure despite the Gnosis Safe misbehaviour outlined. As such, this exhibit is invalid as no wrong data is returned by the function.

c4-judge commented 10 months ago

alex-ppg marked the issue as unsatisfactory: Invalid