Coinbase Flywheel

Description

The Campaign contract accepts native tokens, with ERC-7528 used to identify the native token, as shown below:

// Campaign.soladdress public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;...receive() external payable {}

Funds accumulated in the Flywheel contract are expected to be transferred out when Flywheel invokes Campaign.sendTokens(). However, in every flow where sendTokens() is called, a solvency check is also executed. The issue is that the checks do not support native tokens.

As a result, when they attempt to check solvency using IERC20.balanceOf() on the NATIVE_TOKEN placeholder (0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE), the call reverts since no contract exists at that address, as we can see in the following code snippets:

function _assertCampaignSolvency(address campaign, address token) internal {    uint256 totalAllocated = totalAllocatedPayouts[campaign][token] + totalAllocatedFees[campaign][token];    if (IERC20(token).balanceOf(campaign) < totalAllocated) revert InsufficientCampaignFunds();}

// withdrawFunds()...uint256 requiredSolvency = campaignStatus(campaign) == CampaignStatus.FINALIZED    ? totalAllocatedFees[campaign][token]    : totalAllocatedFees[campaign][token] + totalAllocatedPayouts[campaign][token];if (IERC20(token).balanceOf(campaign) < requiredSolvency) revert InsufficientCampaignFunds();...

This causes the following functions to revert:

• send()
• allocate()
• deallocate()
• distribute()
• distributeFees()
• withdrawFunds()

The impact is that native tokens sent to Campaign contracts become permanently locked and cannot be recovered.

Recommendation

Both _assertCampaignSolvency() and withdrawFunds() should support the native token case, using a native balance check instead of IERC20.balanceOf().

Coinbase

Fixed in 295374.

Cantina

Fixed by implementing the reviewer's recommendation.

Description

When converting a tokenId to a code the normalizeSmallString returns a value that differs from the direct cast to bytes32 (most likely upper/lower case differences) the InvalideCode(code) error is returned. However at this point the code is empty.

It is also confusing that the error indicates the output (code) is incorrect rather than the more typical indication that the input (tokenId) is incorrect.

Recommendation

Either create an additional Error indicating an invalid tokenId and use that or ensure the code is the direct cast of the tokenId or the value returned by fromSmallString.

Base

Will switch lines 293/294

Cantina

Fixed in PR 118

Description

The updatePayoutAddress() function allows the owner of a BuilderCodes token to specify the address that will receive payouts. Since these tokens are transferable, ownership can change. However, when a token is transferred, the payout address does not automatically update. This means the new owner must explicitly set a new payout address; otherwise, payouts may continue to be sent to the previous owner.

Recommendation

Currently, the responsibility of updating the payout address lies entirely with the user. In practice, many users are likely to overlook this step, creating a risk of funds being misdirected. While there is no perfect solution that covers all edge cases, two possible approaches could improve safety:

1. Automatic Update on Transfer
   Override ERC721._update() to automatically call _updatePayoutAddress() with the new owner address.

   • Pro: Ensures the payout address always matches the current owner.
   • Con: If the new owner is a contract that cannot receive payouts, this may lead to failures or lost funds.

2. Maintain Current Design but Improve Awareness
   Leave the logic unchanged but clearly document and communicate the limitation to users and potential buyers.

   • Pro: Avoids compatibility issues with contracts or specialized payout setups.
   • Con: Relies on user diligence, increasing the chance of errors.

Description

The createCampaign() function uses the CREATE2 opcode to deploy new instances of the Campaign contract.
However, the salt used for contract creation does not include msg.sender, which makes the function front-runnable.

An attacker could frontrun a legitimate call to createCampaign() by submitting an identical transaction with the same salt. While the attacker does not gain direct benefits—since the deployed contract would be the same as the intended one—the frontrunning causes the original caller’s transaction to revert. This behavior can lead to several issues:

1. If createCampaign() is called by another contract as part of a broader operation, the entire transaction may revert unexpectedly.
2. A caller may have already pre-computed the address of the new campaign and transferred funds there. If their createCampaign() transaction fails, they may believe their funds are lost.
3. The caller may perceive the failed transaction as a denial-of-service (DoS) attack against their campaign creation attempts.

Recommendation

To mitigate this issue, incorporate msg.sender into the salt when creating the campaign. This ensures uniqueness per caller and prevents front-running collisions:

campaign = Clones.cloneDeterministic(    campaignImplementation,    keccak256(abi.encode(hooks, nonce, hookData, msg.sender)));

## CoinbaseFixed in [295374](https://github.com/base/flywheel/tree/29537488adfbbfefb949c093820cadd906d34cb0).
## CantinaFixed by implementing the reviewer's recommendation.

Description

When payouts to multiple recipients are batched, the complete batch will revert if one of them reverts. For instance this can happen when one of the recipients is on the USDC blocklist.

Note that the manager/attributionProvider does not always know the recipient in advance and thus may not preemptively be able to censor those recipients to avoid the issue. The manager would then have to determine which recipient is causing the issue and resubmit the batch.

Recommendation

Consider returning an array with the status of each of the payments (and fee) rather than revering the batch. Alternatively (or additionally) use some retry mechanism caching the payment info so the user can withdraw when the issue has been resolved.

Base

Fixed by the following combination of PRs:

• PR112: Add failed fee fallback mechanism
• PR121: Make BridgeRewards accept bridged amount in calldata
• PR125: Update BridgeRewards logic (reverts change to using calldata above)

We acknowledge that payouts can block other payouts. We do not think this has the same exposure to DOS risk.

Cantina

Fixed for the fee distribution.

Description

The BridgeRewards hook contract limits the maximum feeBps that can be applied. If this is exceeded the payout reverts which causes the user's funds to be locked in the bridge until the issue is resolved.

Recommendation

Consider capping the fee to it's maximum rather than reverting in this case so the user immediately receive their funds.

Cantina

Fixed in PR 116

Description

When an incorrect builder code is used while bridging user funds the transaction reverts and the user are locked in the bridge until the bridge operator resolves the issue.

Recommendation

An incorrect builder token is the bridge's error and should not lead to user inconvenience. For better UX consider putting the inconvenience on the builder by handling it as a payout without builder code and transfering all of the funds to the user.

Cantina

Fixed in PR 116

Description

The constructor of AdConversion takes an owner_ as parameter but it is never used and the contract isn't Ownable as there is no owner-only functionality provided by the contract.

Recommendation

Remove the owner_ from the constructor definition.

Base

Fixed in PR115

Cantina

Fixed

Description

When Flywheelcreates a new campaign and calls onCreateCampaign() on the CashbackRewards, SimpleRewards and AddConversion hook contracts they do not check if the campaign is already registered, in which case they should revert instead of silently overwriting the existing campaign data as they currently do.

Although the Flywheel contract always generates a new campaign address CashbackRewards, SimpleRewards and AddConversion should not rely on how they are used to perform correctly.

Also note that due to versioning of Flywheel the need may arise for hook contracts to support multiple instances of (different versions of) Flywheel. Therefor it would make sense to add the flywheel address as a parameter of the internal hook functions like _onCreateCampaign().

Recommendation

Consider checking whether the indicated campaign address is already regstered and revert in case it is.

Additionally consider modifying the internal interface to include the calling flywheel contract to allow the implementation to separate campaign storage per instance of flywheel. Although msg.sender can always be used for this it would make for a cleaner integration as 3rd party implementers are encouraged to inherit from CampaignHooks.

Base

Acknowledged

Cantina Managed

Acknowledged

Description

The referral codes generated for consecutive registrations could be swapped between two users when a block is reorged and the order of their transactions is swapped

The code generated is based on the following parameters:

• nonce (sequence number tracked by the register function)
• block timestamp
• block hash of the previous block
• prevrandao

In case of a reorg prevrandao can be the same for the original block and the reorged block. On some chains like Arbitrum prevrandao is always a fixed value. The block timestamp of both blocks has a high probability of being equal and the block hash of the previous block will be the same for the first reorged block.

Therefor if the order of two register transaction are swapped users could end up with swapped codes after the reorg versus what they received in their original confirmation.

Recommendation

Although this is en extreme edge case you might consider to further reduce the likelihood of this happening by including msg.sender, tx.origin. As these still do not offer any guarantee (for third party contracts and account abstraction) adding a (non-sequential) randomness parameter to the generate() function can additionally be considered for these situations.

Base

I think we’re okay with this risk. Thanks for callout

Cantina

Acknowledged

Description

The BuilderCodes.updateMetadata function is missing _requireOwned(tokenId) check due to which a MetadataUpdate event can be emitted for a non-existent tokenId or a tokenId which will be minted in future.

Recommendation

function updateMetadata(uint256 tokenId) external onlyRole(METADATA_ROLE) {+   _requireOwned(tokenId);    emit MetadataUpdate(tokenId);}

Description

These constant variables have misplaced n and o characters:

1. ALLOWED_CHARACTERS in BuilderCodes
2. ALPHANUMERIC in PseudoRandomRegistrar

Recommendation

Switch the places of n and o characters.

- 0123456789abcdefghijklmonpqrstuvwxyz+ 0123456789abcdefghijklmnopqrstuvwxyz

Description

The Flywheel.createCampaign() function allows callers to specify an arbitrary hooks contract.
This introduces a potential risk: if the specified hook contract writes to its own storage inside _onCreateCampaign(), the stored data may accumulate over time.

If other functions in the system later iterate over this storage, the increasing data size could cause those calls to run out of gas and revert, effectively leading to a denial of service.

Recommendation

Document this potential pitfall and clearly warn implementers of hook contracts to avoid unbounded storage writes in _onCreateCampaign().

Description

This repository does not include a foundry.lock file, which ensures deterministic builds by locking dependency versions. Without it, contributors may build with different dependency versions, risking inconsistent behavior or security issues.

Proof-of-impact

Recent npm supply-chain attacks showed how compromised packages can introduce wallet-hijacking malware. Unlocked dependencies can expose projects to similar risks.

Recommendation

Commit foundry.lock to version control and pin dependencies to ensure reproducible and secure builds.

Coinbase

Fixed in 295374

Cantina

Fixed by implementing the reviewer's recommendation.

Description

When updating a hook contract's metadata the updated URI is queried in a separate call in campaignURI This is an unnecessary call as the onUpdateMetadata() function could return the URI immediately.

Recommendation

Consider modifying onUpdateMetadata() to reurn the newly modified URI in order to save the cost of an extra call to the same hook contract.

Base

Discussed and will leave it as is.

Cantina

Acknowledged

Description

// CashbackRewards.solimport {CampaignHooks} from "../CampaignHooks.sol";

// Flywheel.solimport {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

CampaignHooks is not used in CashbackRewards and SafeERC20 is not used in Flywheel.

Recommendation

Consider removing these unused imports to reduce the contract size.