Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Lack of a refund mechanism for users when the callExternalContract function encounters an error.

kevinkkien

Summary

The callExternalContract function in the contract lacks a mechanism to refund the amount (msg.value) sent by the caller in case the external contract does not behave as expected or if the caller does not have sufficient token balance to complete the transaction. This could result in users losing their funds without a refund if the external contract call fails.

Vulnerability Details

The callExternalContract function makes a call to an external contract using the call method. During this process, the function sends an amount of Ether determined by msg.value from the caller to the external contract. However, if the external contract does not behave as expected (e.g., due to an error in the external contract or a failed contract execution), the transaction will be reverted without a mechanism to refund the amount sent. This introduces a risk where the caller could lose the funds they sent in the transaction without receiving a refund.

  • No refund in case of failure: When the call to the external contract fails (i.e., the success variable is false), the require(success, "External call failed") will revert the transaction, but there is no measure in place to refund the msg.value to the caller.

  • Loss of funds on failure: The caller may lose the Ether they sent if the external contract does not behave as expected or encounters an issue, without any refund mechanism in place.

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L218-L221

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/MembershipFactory.sol#L176-L180

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

Impact

The user loses money if the function fails.

Tools Used

Manual

Recommendations

Use Solidity's try/catch mechanism to catch and handle errors from external contracts. When an error occurs, the current contract can automatically refund the amount sent to the caller.

function callExternalContract(address contractAddress, bytes memory data) external payable onlyRole(OWP_FACTORY_ROLE) returns (bytes memory) {
(bool success, bytes memory returndata) = contractAddress.call{value: msg.value}(data);
if (!success) {
// Refund the caller if the external contract call fails.
payable(msg.sender).transfer(msg.value);
revert("External call failed");
}
return returndata;
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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