Insufficient validation of contracts when setting authorised address.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/AddressProvider.sol#L77-L90 https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/AddressProvider.sol#L84 https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/AddressProvider.sol#L131

Vulnerability details

Impact

governance can set Malicious contract as authorised address and since the AddressProvider.sol is a singular source of truth an attacker can craft an exploit to abuse _authorizedAddress privileges.

Proof of Concept

A miniaturised POC is shown below.

in the AddressProvider.sol when setAuthorizedAddress(...) is called to set _authorizedAddress as an authorised address with _overrideCheck set to false, the checks performed before updating the state is not sufficient to ensure that a malicious contract is not added as an authorised address.

    function setAuthorizedAddress(bytes32 _key, address _authorizedAddress, bool _overrideCheck) external {
        ...

        /// @dev skips checks for supported `addressProvider()` if `_overrideCheck` is true
>        if (!_overrideCheck) {
            /// @dev skips checks for supported `addressProvider()` if `_authorizedAddress` is an EOA
>           if (_authorizedAddress.code.length != 0) _ensureAddressProvider(_authorizedAddress);
        }

        authorizedAddresses[_key] = _authorizedAddress;

        ...
    }

the _ensureAddressProvider(...) deploys the IAddressProviderService at the _authorizedAddress thereby handing over control to the _authorizedAddress as shown below.

   function _ensureAddressProvider(address _newAddress) internal view {
       if (IAddressProviderService(_newAddress).addressProviderTarget() != address(this)) {
           revert AddressProviderUnsupported();
       }
   }

POC

An attacker can exploit this vulnerability either directly or by proxy

1. Proxy Scenario is

   • An attacker uses his Malicious proxy contract with a fallback and deploys a v1 implementation contract at address X that implements an addressProviderTarget() function that points to or returns address(addressProvider).
   • After successfully authorising the malicious contract, attacker upgrades implementation contract to v2 at address Y that implements a addressProviderTarget() function that points to or returns a different address.

2. Direct Scenario is shown in the coded POC below

   • Attacker implements addressProviderTarget() function in his malicious contract that points to or returns address(addressProvider) but has some malicious code in the function and other malicious functions in his contract as shown in the POC below
   • Add the POC to the /2023-10-brahma/contracts/test/branch-trees/AddressProvider/set-authorized-address/SetAuthorizedAddress.t.sol and
   • run forge test -vv --match-test testSetAuthorisedMaliciousAddress --fork-url $MAINNET_RPC

contract AuthorisedMaliciousAddress is Test, ConsoleFactory("offchain/addressManager.ts") {

    function setUp() public {
        ConsoleFactory.deployConsole(address(this), false);
    }

    // retrun AddressProviders address on callback into this malicious contract
    function addressProviderTarget() public view returns(address) {
        // other malicious actions can be performed before the return
        console2.log("Callback successfully made into malicious address");
        return  address(addressProvider);
    }

    function testSetAuthorisedMaliciousAddress() public {
        bytes32 _key = keccak256(abi.encode(address(this)));
        addressProvider.setAuthorizedAddress(_key, address(this), false);
        console2.log("Malicious address was successfully set as authorised address");
    }

    // other malicious code to be executed after authorisation has been granted
}

Tools Used

Foundry

Recommended Mitigation Steps

Consider implementing a whitelist of authorised contract OR add a delay before authorised contracts are added by governance.

Assessed type

Invalid Validation

[0xad1onchain]

Invalid, governance abuse is out of scope

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #39

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

As the Sponsor has correctly specified, governance abuse is considered out-of-scope in this contest. While the Warden's rationale is somewhat correct, it would require the governance member to collude and set a potentially malicious contract in the authorized address list.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[c4-judge]

alex-ppg marked the issue as primary issue

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid