0xadrii - Incentives might be applied twice in certain situations

[sherlock-admin]

0xadrii

medium

Incentives might be applied twice in certain situations

Summary

Incentives will be applied more than one time in _checkAndApplyIncentives() if more than one account has an incentives contract set in the incentiveContract mapping.

Vulnerability Detail

One of the mechanisms to keep uAD’s peg to $1 is to add incentives when the price of uAD goes below $1. In such situation, traders on the 3CRV pool will be incentivized to buy the tokens and disincentivized to sell the token. This is achieved by adding a _checkAndApplyIncentives() function when performing uAD token transfers:

function _checkAndApplyIncentives(
        address sender,
        address recipient,
        uint256 amount
    ) internal {
        // incentive on sender
        address senderIncentive = incentiveContract[sender];
        if (senderIncentive != address(0)) {
            IIncentive(senderIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
        }

        // incentive on recipient
        address recipientIncentive = incentiveContract[recipient];
        if (recipientIncentive != address(0)) {
            IIncentive(recipientIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
        }

        // incentive on operator
        address operatorIncentive = incentiveContract[_msgSender()];
        if (
            _msgSender() != sender &&
            _msgSender() != recipient &&
            operatorIncentive != address(0)
        ) {
            IIncentive(operatorIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
        }

        // all incentive, if active applies to every transfer
        address allIncentive = incentiveContract[address(0)];
        if (allIncentive != address(0)) {
            IIncentive(allIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
        }
    }

As shown in the code snippet, the sender, recipient, _msSender() and address(0) are all susceptible of having an incentives contract in the incentiveContract mapping set for them to be incentivized.

Following Ubiquity’s documentation: “Incentivization is achieved by burning a percentage of all uAD sold and issuing governance tokens (UBQ) in equal proportions for all uAD bought”. This is translated to code in the LibCurveDollarIncentive.sol , where buys or sells will be incentivized depending on who is the sender/receiver:

// LibCurveDollarIncentive.sol
function incentivize(
        address sender,
        address receiver,
        uint256 amountIn
    ) internal {
        require(sender != receiver, "CurveIncentive: cannot send self");

        if (sender == LibAppStorage.appStorage().stableSwapMetaPoolAddress) {
            _incentivizeBuy(receiver, amountIn);
        }

        if (receiver == LibAppStorage.appStorage().stableSwapMetaPoolAddress) {
            _incentivizeSell(sender, amountIn);
        }
    }

The problem with the incentives approach shown in the first code snippet is that more than one incentivize calls might take place in a single transfer, which will make the final outcome of the incentives be larger than what it is expected to be. As we can see in the second snippet, the incentivize() function will always perform the same operation, and given that _checkAndApplyIncentives() will call incentivize() with the same parameters regardless of who is the incentivized user (sender, recipient, _msSender() and address(0)), several situations might arise where incentivize() gets called multiple times and the incentives are too large.

An example of a situation where incentives would be applied wrong would be if sender, _msSender() and address(0) all had an incentives contract set in the incentiveContract mapping. If this occured, incentivize() would be wrongly called three times (once for sender , once for _msSender() and once for address(0)), instead of actually applying the incentive only once.

Impact

I consider this vulnerability to be of medium impact. Incentives will be triggered more than once, effectively making one of the mechanisms to stabilize peg to perform in a wrong way, making senders and receivers obtain more rewards in UBQ tokens than they should be entitled.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/core/UbiquityDollarToken.sol#L92-L136

Tool used

Manual Review

Recommendation

Consider returning after an incentivize() is triggered in _checkAndApplyIncentives(). This will make incentives just be triggered once:

function _checkAndApplyIncentives(
        address sender,
        address recipient,
        uint256 amount
    ) internal {
        // incentive on sender
        address senderIncentive = incentiveContract[sender];
        if (senderIncentive != address(0)) {
            IIncentive(senderIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
+            return;
        }

        // incentive on recipient
        address recipientIncentive = incentiveContract[recipient];
        if (recipientIncentive != address(0)) {
            IIncentive(recipientIncentive).incentivize(
                sender,
                recipient, 
                _msgSender(),
                amount
            );
+           return;
        }

        // incentive on operator
        address operatorIncentive = incentiveContract[_msgSender()];
        if (
            _msgSender() != sender &&
            _msgSender() != recipient &&
            operatorIncentive != address(0)
        ) {
            IIncentive(operatorIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
+            return;
        }

        // all incentive, if active applies to every transfer
        address allIncentive = incentiveContract[address(0)];
        if (allIncentive != address(0)) {
            IIncentive(allIncentive).incentivize(
                sender,
                recipient,
                _msgSender(),
                amount
            );
+           return;
        }
    }

Duplicate of #8