ReceiveAllGatheredFundsFacet Manual Review Findings

RAG-01M: Incorrect State Transition

Type	Severity	Location
Logical Fault		ReceiveAllGatheredFundsFacet.sol:L30

Description:

The referenced state transition is considered insecure as it will attempt to increase the state by one which might not always advanced to the desired post-receive state.

Impact:

The receive mechanism is incompatible with the type(uint256).max state due to an overflow error and generally relies on an assumption that can be trivially breached.

Example:

Copy
21function doReceive(uint256 campaignId, address account) external onlyInternalDelegateCall {
22    FungibleAndSemiFungiblePurchaseStorage.IdInfo memory info = FungibleAndSemiFungiblePurchaseStorage.layout().infoForId[
23        campaignId
24    ];
25    IERC20(info.fundingCurrency).safeTransfer(account, info.totalFundsGathered);
26
27    // The change of state here is extremely important, maybe it should be a different facet.
28    // The importance comes from the fact that if state does not change, receive can happen over and over again!
29    uint256 currentState = IStateFacet(address(this)).getStateOfId(campaignId);
30    IStateFacet(address(this)).changeState(campaignId, currentState, currentState + 1);
31
32    emit DoReceiveExecuted(campaignId, account);
33}

Recommendation:

We advise the system to reserve the 8 top bits of the state IDs by restricting state configurations to the uint248 data type, permitting special states such as buy-backs and post-receives to be represented as the maximum value of the uint256 data type and downward through constant declarations.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

The system utilizes a storage-configured post-receive state in the latest implementation, permitting the Evergon Labs team to define a specific state that is considered to represent the "post-receive" state of a campaign thus addressing this exhibit.

RAG-02M: Inexistent Validation of Caller Authorization

Type	Severity	Location
Logical Fault		ReceiveAllGatheredFundsFacet.sol:L21-L33

Description:

The ReceiveAllGatheredFundsFacet::doReceive function does not apply any validation to its caller and the ReceiveSkeleton and ReceiveSkeletonNID contracts that integrate with it do not apply any either.

Impact:

Any caller can claim the raised funds of a campaign as long as it has achieved the post-funding state mandated by the SingleStateReceiveFacet::checkReceiveState function.

Example:

Copy
21function doReceive(uint256 campaignId, address account) external onlyInternalDelegateCall {
22    FungibleAndSemiFungiblePurchaseStorage.IdInfo memory info = FungibleAndSemiFungiblePurchaseStorage.layout().infoForId[
23        campaignId
24    ];
25    IERC20(info.fundingCurrency).safeTransfer(account, info.totalFundsGathered);
26
27    // The change of state here is extremely important, maybe it should be a different facet.
28    // The importance comes from the fact that if state does not change, receive can happen over and over again!
29    uint256 currentState = IStateFacet(address(this)).getStateOfId(campaignId);
30    IStateFacet(address(this)).changeState(campaignId, currentState, currentState + 1);
31
32    emit DoReceiveExecuted(campaignId, account);
33}

Recommendation:

We advise proper caller authorization to be imposed on either the ReceiveAllGatheredFundsFacet implementation or its parent callers, ensuring that the correct party acquires all raised funds of a particular campaignId.

Alleviation (71cda4ccfdcfa25fb96a4565f1f8143b350dd246):

All top-level callers of the ReceiveAllGatheredFundsFacet::doReceive function have been updated to impose proper access control, alleviating this exhibit as a result.