Project

One World
NFTDeFi
15,000 USDC
View results
Submission Details
Severity: medium
Invalid

Access Control

## Summary

The EXTERNAL_CALLER role is used to allow external contract calls, but it may not be properly controlled or restricted. Unauthorized addresses might exploit this functionality to execute external calls, compromising the contract’s integrity.

## Finding Description

The issue is with the access control mechanism for the EXTERNAL_CALLER role in the callExternalContract function. While the function is restricted by the onlyRole(EXTERNAL_CALLER) modifier, the security of the role assignment depends on the proper management of the EXTERNAL_CALLER role itself. If this role is not properly protected, it may be granted to unauthorized addresses, enabling malicious actors to call sensitive functions that interact with external contracts.

Security Guarantees Broken:

  • Authorization Control: Malicious actors could exploit the access control system to interact with external contracts and perform unauthorized actions.

  • Integrity: Unintended users could execute arbitrary external calls, leading to potential vulnerabilities such as data corruption or reentrancy attacks.

## Vulnerability Details

The main problem arises when the EXTERNAL_CALLER role is not securely assigned or properly restricted to trusted addresses. Since this role grants permission to perform external contract calls (callExternalContract), a breach of the access control could allow malicious entities to trigger actions in external contracts, potentially leading to:

  • Reentrancy attacks: Malicious contracts might re-enter and modify the state unexpectedly.

  • Data leakage or manipulation: Unauthorized external calls could leak sensitive data or modify the contract state in unexpected ways.

## Impact

The impact of this vulnerability is moderate to high, depending on the external contracts that are called and their interactions with the MembershipFactory contract. If attackers gain control over the EXTERNAL_CALLER role, they can trigger malicious external contract calls, potentially stealing assets or corrupting contract state.

  • Severe if external contract calls involve asset transfers or state changes.

  • Moderate if external calls are limited to non-sensitive operations.

## Proof of Concept

Here’s a conceptual proof of how the issue can be exploited:

  1. Step 1: Malicious actor gains unauthorized access to the EXTERNAL_CALLER role.

  2. Step 2: Using this role, the attacker calls the callExternalContract function.

  3. Step 3: The attacker can execute arbitrary code in external contracts that could leak information or change the state of the contract.

Example:

// A malicious contract that can be invoked by the attacker
contract MaliciousContract {
MembershipFactory factory;
constructor(address _factory) {
factory = MembershipFactory(_factory);
}
// Exploits the access control issue to call external functions
function exploit(address _targetContract, bytes memory data) external {
factory.callExternalContract(_targetContract, data);
}
}

If this malicious contract gains the EXTERNAL_CALLER role, it can execute arbitrary external calls.

## Recommendations

To resolve this issue, the access control system should be improved to ensure the EXTERNAL_CALLER role is only granted to trusted addresses and cannot be manipulated.

Fix 1: Restrict Role Assignment

Add an additional check or restrict the ability to assign the EXTERNAL_CALLER role. The grantRole function should only be callable by trusted administrators or should have more strict validation.

For example:

function grantExternalCallerRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
require(account != address(0), "Invalid address");
_grantRole(EXTERNAL_CALLER, account);
}

This way, only admins can assign the EXTERNAL_CALLER role.

Fix 2: Add Access Control to External Calls

Ensure that callExternalContract is only accessible by trusted users with the correct role and permissions. Consider adding multi-signature or multi-layered checks to reduce risk further.

For example, improve the callExternalContract function with an additional check:

function callExternalContract(address contractAddress, bytes memory data) external onlyRole(EXTERNAL_CALLER) returns (bytes memory) {
require(hasPermissionToCallExternal(contractAddress), "Unauthorized contract call");
(bool success, bytes memory returndata) = contractAddress.call{value: msg.value}(data);
require(success, "External call failed");
return returndata;
}

Where hasPermissionToCallExternal could validate that only trusted addresses are allowed to make external calls.

## File Location:

MembershipFactory.sol

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.