6.1 Incentra incentives with CROSS_CHAIN rewards campaign can never be created
Incentra incentives support two types of campaigns: SAME_CHAIN and CROSS_CHAIN, depending on where the rewards are submitted. When a new incentive campaign is created, the processIncentiveCampaignCreation hook is invoked. This hook verifies that the externalPayoutAddress in the Incentra campaign is set to the IncentraAV contract address:
function processIncentiveCampaignCreation(
bytes32 _incentiveCampaignId,
address[] memory _incentivesOffered,
uint256[] memory _incentiveAmountsOffered,
bytes memory _actionParams,
address /*_ip*/
)
external
onlyIncentiveLocker
{
// Decode the campaign params
ActionParams memory params = abi.decode(_actionParams, (ActionParams));
// ...
// Check that the AV can process claims and refunds for this campaign
require(IIncentraCampaign(params.incentraCampaign).externalPayoutAddress() == address(this), IncentraPayoutAddressMustBeAV());
// ...
}
However, only campaigns of type SAME_CHAIN in the Brevis contracts expose the public externalPayoutAddress function. For CROSS_CHAIN campaigns, the external payout address is instead encoded in side the config struct and not accessible via the same interface. This misalignment causes the processIncentiveCampaignCreation function to revert for every CROSS_CHAIN campaign, effectively making it impossible to create such campaigns.
Recommendation
Consider handling the retrieval of externalPayoutAddress differently for campaigns of type CROSS_CHAIN.
Resolution
Fixed on Incentra contracts: 055c33c
6.2 Malicious Incentive Provider can block reward claims by transferring Points Program ownership to the zero address
The claimIncentives(...) function in the IncentiveLocker contract allows an Action Provider (AP) to claim rewards from an incentive campaign. These rewards can be standard ERC20 tokens or non-transferable points from a Points Program. The contract iterates through all owed incentives and distributes them.
The _pushIncentivesAndAccountFees(...) function uses the isPointsProgram(incentive) check to distinguish between an ERC20 token and a Points Program. If this returns true, it calls the internal _award(...) function. If it returns false, it assumes the incentive is an ERC20 token and performs a safeTransfer(...).
function _pushIncentivesAndAccountFees(
address incentive,
address ap,
// ...
) internal {
// @audit Checks if the incentive is a Points Program.
if (isPointsProgram(incentive)) {
// ...
_award(incentive, ap, incentiveAmount);
} else {
// ...
// @audit If isPointsProgram returns false, it assumes `incentive` is an ERC20.
ERC20(incentive).safeTransfer(ap, incentiveAmount);
}
}
The isPointsProgram(...) function, inherited from PointsRegistry, determines a program's existence by checking if it has a non-zero owner address.
function isPointsProgram(address _pointsId) public view returns (bool exists) {
// @audit-issue Existence is tied to the owner not being the zero address.
exists = pointsIdToProgram[_pointsId].owner != address(0);
}
The vulnerability arises because the transferPointsProgramOwnership(...) function in the PointsRegistry contract lacks a check to prevent transferring ownership to the zero address.
function transferPointsProgramOwnership(address _pointsId, address _newOwner) external {
PointsProgram storage pointsProgram = pointsIdToProgram[_pointsId];
require(pointsProgram.owner == msg.sender, OnlyPointsProgramOwner());
// @audit-issue The `_newOwner` can be `address(0)`, effectively disabling the program.
pointsProgram.owner = _newOwner;
emit PointsProgramOwnershipTransferred(_pointsId, _newOwner);
}
A malicious Incentive Provider (IP) can exploit this to mount a denial-of-service attack. An IP can create a campaign offering a legitimate ERC20 token and a Points Program they own as incentives. After an AP becomes eligible for rewards, but before they claim them, the IP can call transferPointsProgramOwnership(...) and set the new owner of their Points Program to address(0).
The transaction will revert when the AP attempts to claim their rewards via claimIncentives(...). The isPointsProgram(...) check will return false for the Points Program incentive, causing the contract to incorrectly treat its address as an ERC20 token and call safeTransfer(...) on it. This call will fail, reverting the entire transaction and preventing the AP from claiming any of their rewards, including the legitimate ERC20 tokens.
Recommendation
Consider adding a validation check in the transferPointsProgramOwnership(...) function to prevent transferring ownership of a Points Program to the zero address. Alternatively, the isPointsProgram(...) check could be modified to rely on an existing program property, which can't be a zero value or a dedicated isCreated flag, rather than the mutable owner address.
6.3 Passing a non-existent incentive with zero amount incorrectly removes the last incentive
The removeIncentives(...) function allows an Incentive Provider (IP) or a Co-Incentive Provider (Co-IP) to remove incentives from an existing campaign. The function iterates through a provided list of incentives and their corresponding amounts to remove.
The issue's core lies in the fact that the function does not validate whether the incentive being removed is part of the campaign. If an IP provides a non-existent incentive address along with a _incentiveAmountsToRemove of 0, the logic proceeds as if it's a valid operation. Specifically, the ics.incentiveToAmountOffered[incentive] for a non-existent incentive is 0, and subtracting the provided 0 amount results in 0. This incorrectly triggers the condition ics.incentiveToAmountOffered[incentive] == 0, leading to a call to the internal _removeIncentiveFromCampaign(...) function.
function removeIncentives(
bytes32 _incentiveCampaignId,
address[] memory _incentivesToRemove,
uint256[] memory _incentiveAmountsToRemove,
bytes memory _removalParams,
address _recipient
) public nonReentrant {
// ...
for (uint256 i = 0; i < numIncentives; ++i) {
address incentive = _incentivesToRemove[i];
// ...
uint256 incentiveAmountRemoved = _incentiveAmountsToRemove[i];
if (incentiveAmountRemoved >= ics.incentiveToAmountRemaining[incentive]) {
// ...
} else {
// ...
}
ics.incentiveToAmountOffered[incentive] -= incentiveAmountRemoved;
// @audit If a non-existent incentive and a zero amount are passed, ics.incentiveToAmountOffered[incentive] will be 0.
if (ics.incentiveToAmountOffered[incentive] == 0) {
// @audit-issue This function is called with a non-existent incentive.
_removeIncentiveFromCampaign(ics, incentive);
}
// ...
}
// ...
}
The _removeIncentiveFromCampaign(...) function is then called with this non-existent incentive. Inside this function, it attempts to find the provided incentive in the _ics.incentivesOffered array. Since the incentive does not exist, the loop to find its index will complete without finding a match. The function then unconditionally calls _ics.incentivesOffered.pop(), which removes the *last* incentive from the array, regardless of what it is.
function _removeIncentiveFromCampaign(ICS storage _ics, address _incentive) internal {
uint256 lastIndex = _ics.incentivesOffered.length - 1;
uint256 index = 0;
// @audit This loop will not find the non-existent _incentive.
for (index; index < lastIndex; ++index) {
if (_ics.incentivesOffered[index] == _incentive) break;
}
if (index != lastIndex) {
// ...
}
// @audit-issue The last element is popped, even if _incentive was not found.
_ics.incentivesOffered.pop();
}
Recommendation
Consider ensuring that an incentive exists within the campaign before attempting to process its removal.
