Protocol - Irrigation

The AuctionUpgradeable contract allows creating different auctions through the createAuction() function:

AuctionUpgradeable.sol

function createAuction(
    uint96 startTime,
    uint96 duration,
    address sellToken,
    uint256 trancheIndex,
    uint128 sellAmount,
    uint128 minBidAmount,
    uint128 fixedPrice,
    uint128 priceRangeStart,
    uint128 priceRangeEnd,
    AuctionType auctionType
) external payable returns (uint256)

Once an auction is completed, the closeAuction() function will be called and will distribute the sellToken between all the bidders. As currently USDC contains a blacklist of users which cannot transfer or receive this token, the following issue could occur:

1. Alice creates an auction of 1000 USDC(sellToken).
2. Users place different bids for the USDC auction.
3. A Blacklisted USDC user also places a bid for the USDC auction. The bid is placed using a whitelisted purchaseToken like DAI.
4. The auction is completed. closeAuction() is called, but it reverts every time as the USDC transfer to the blacklisted USDC user is forbidden.

5. The auction USDC sellTokens are now stuck in the contract. All the bids are also stuck permanently in the contract.

6. Similar issue can occur with USDT as it also contains a blacklist of users.

SOLVED: The Irrigation Protocol team solved the issue by enforcing a whitelist on the sellTokens. The whitelist will not include blacklistable tokens like USDC/USDT.

Commit ID : cae1d1718ef504f4fc27ba1aa464600a71ae635a.

As mentioned before, the AuctionUpgradeable contract allows creating different auctions through the createAuction() function:

AuctionUpgradeable.sol

function createAuction(
    uint96 startTime,
    uint96 duration,
    address sellToken,
    uint256 trancheIndex,
    uint128 sellAmount,
    uint128 minBidAmount,
    uint128 fixedPrice,
    uint128 priceRangeStart,
    uint128 priceRangeEnd,
    AuctionType auctionType
) external payable returns (uint256)

The minBidAmount parameter sets the minimum amount of the token that can be bid. In the case that the auctioneer sets the minBidAmount to a very low value, i.e. 1, the following attack vector would be possible:

1. Alice (auctioneer) creates an auction of 1000 USDC tokens and sets minBidAmount to 1. The auctionType is a TimedAuction.
2. Bob calls placeBid(1, 1, DAI, 1000e18) bidding 0.01 DAI for 0.000001 USDC. (bidPrice = 1000 DAI).
3. Joe calls placeBid(1, 100e6, DAI, 1e18) bidding 100 DAI for 100 USDC. (bidPrice = 1 DAI) but it reverts with a low Bid error.
4. If any user wants to bid now, they should place a bid higher than the abusive bid placed by Bob. (bidPrice = 1000 DAI). Bob managed to cause a Denial of Service, losing only 0.01 DAI.

SOLVED: The Irrigation Protocol team solved the issue by implementing the recommended solution.

Commit ID : 0a94a93c0f191c992e9f75a41d630ae3dbf80de8.

In the ChainlinkOracle contract, the function getChainlinkPrice() is used to retrieve prices from different Chainlink aggregators:

ChainlinkOracle.sol

/// @dev returns price with decimals 18
function getChainlinkPrice(AggregatorV2V3Interface feed) internal view returns (uint256) {
    // Chainlink USD-denominated feeds store answers at 8 decimals
    uint256 decimalDelta = uint256(18) - feed.decimals();
    // Ensure that we don't multiply the result by 0
    if (decimalDelta > 0) {
        return uint256(feed.latestAnswer()) * 10 ** decimalDelta;
    } else {
        return uint256(feed.latestAnswer());
    }
}

According to the Chainlink's documentation this function is deprecated and should not be used. Moreover, using latestAnswer() could lead to return invalid/stale prices.

SOLVED: The Irrigation Protocol team solved the issue by implementing the recommended solution.

Commit ID : 57d3945089a52425b91cdc8005b62b73d5648c68.

In the AuctionUpgradeable contract, the internal function _settleAuction() is called every time an auction is closed:

AuctionUpgradeable.sol

function _settleAuction(uint256 auctionId) internal {
    AuctionData memory auction = AuctionStorage.layout().auctions[auctionId];
    uint256 trancheIndex = auction.trancheIndex;
    // when there are no bids, all token amount will be transfered back to seller
    if (auction.curBidId == 0) {
        if (trancheIndex == 0) {
            TransferHelper.safeTransfer(auction.sellToken, auction.seller, auction.reserve);
        } else {
            IERC1155Upgradeable(address(this)).safeTransferFrom(
                address(this),
                auction.seller,
                trancheIndex,
                auction.reserve,
                Constants.EMPTY
            );
        }

        AuctionStorage.layout().auctions[auctionId].reserve = 0;
        AuctionStorage.layout().auctions[auctionId].status = AuctionStatus.Closed;
        emit AuctionClosed(auction.reserve, auctionId);
        return;
    }
    uint128 availableAmount = auction.reserve;
    uint256 settledBidCount = 0;
    uint256 curBidId = auction.curBidId;
    do {
        Bid memory bid = AuctionStorage.layout().bids[auctionId][curBidId];
        uint128 settledAmount = _settleBid(
            auction.sellToken,
            auction.trancheIndex,
            auction.seller,
            bid,
            availableAmount
        );
        availableAmount -= settledAmount;
        AuctionStorage.layout().bids[auctionId][curBidId].bCleared = true;
        --curBidId;
        ++settledBidCount;
    } while (
        curBidId > 0 &&
            settledBidCount <= MAX_CHECK_BID_COUNT &&
            availableAmount >= auction.minBidAmount
    );
    if (availableAmount > 0) {
        if (auction.assetType == AssetType.ERC20) {
            TransferHelper.safeTransfer(auction.sellToken, auction.seller, availableAmount);
        } else {
            IERC1155Upgradeable(address(this)).safeTransferFrom(
                address(this),
                auction.seller,
                trancheIndex,
                availableAmount,
                Constants.EMPTY
            );
        }
    }

    AuctionStorage.layout().auctions[auctionId].reserve = 0;
    AuctionStorage.layout().auctions[auctionId].status = AuctionStatus.Closed;
    emit AuctionClosed(availableAmount, auctionId);
}

This function iterates over all the bids, starting with the last one, transferring the purchased tokens to the bidder until reaching MAX_CHECK_BID_COUNT (which is hardcoded to 200). The rest of the bids will be automatically cancelled.

Based on this, the following attack vector could be possible:

1. Alice creates an auction:

AuctionUpgradeable(irrigationdiamond).createAuction(
    uint96(block.timestamp), 
    uint96(86400 * 7), 
    address(USDC), 
    0, 
    1000e6, 
    1, 
    1e18, 
    1, 
    2e18, 
    AuctionType.TimedAuction
);

1. Different users place different bids. In total, 150 bids were placed.
2. Alice is not happy with the current bid amounts, so she places 200 different bids on her own auction, 1 second before the auction is completed.
3. The auction is completed. Alice gets her 200 bids, although the rest of the bids of the other users placed before would be automatically canceled.

SOLVED: The Irrigation Protocol team solved the issue by implementing the recommended solution.

Commit ID : c269697ae4b7997abec4c836d46e3f73f292af03.

In the WaterTowerUpgradeable contract, the function _swapEthForWater() is used to swap Ether for Beans using the Curve Router:

WaterTowerUpgradeable.sol

function _swapEthForWater(uint256 amount) internal returns (uint256 waterAmount) {
    if (WaterTowerStorage.layout().middleAssetForIrrigate == Constants.BEAN) {
        /// @dev swap ETH for BEAN using curve router
        address[9] memory route = [
            Constants.ETHER,
            Constants.TRI_CRYPTO_POOL,
            Constants.USDT,
            Constants.CURVE_BEAN_METAPOOL,
            Constants.BEAN,
            Constants.ZERO,
            Constants.ZERO,
            Constants.ZERO,
            Constants.ZERO
        ];
        uint256[3][4] memory swapParams = [
            [uint(2), 0, 3],
            [uint(3), 0, 2],
            [uint(0), 0, 0],
            [uint(0), 0, 0]
        ];
        uint256 beanAmount = ICurveSwapRouter(Constants.CURVE_ROUTER).exchange_multiple{
            value: amount
        }(route, swapParams, amount, 0);

        waterAmount = ISprinklerUpgradeable(address(this)).getWaterAmount(
            Constants.BEAN,
            beanAmount
        );
    }
}

This function is calling the exchange_multiple function with no prevention against slippage:

CURVE_ROUTER contract

@external
@payable
def exchange_multiple(
    _route: address[9],
    _swap_params: uint256[3][4],
    _amount: uint256,
    _expected: uint256,
    _pools: address[4]=[ZERO_ADDRESS, ZERO_ADDRESS, ZERO_ADDRESS, ZERO_ADDRESS],
    _receiver: address=msg.sender
) -> uint256:

Based on this, any autoIrrigate() or irrigate() call could be sandwiched. A malicious user could be monitoring the mempool for any autoIrrigate() or irrigate() call and:

1. Front-run, the autoIrrigate() or irrigate() call to buy beans from the Curve pool. This pushes the price of Bean in the pool up.
2. autoIrrigate() or irrigate() call is executed and Beans are bought at a higher price. This raises, once again, the Bean price.
3. The malicious user back-runs the autoIrrigate() or irrigate() call and sells the Beans at a higher price for a profit.

SOLVED: The Irrigation Protocol team solved the issue by implementing the recommended solution.

Commit ID : f573cdc385d11a8ab3b8929e97678ace5221e4d7.

In the WaterTowerUpgradeable contract, the function autoIrrigate() is used to auto-compound the user rewards:

WaterTowerUpgradeable.sol

function autoIrrigate(address user, uint256 rewardAmount) external onlySuperAdminRole {
    if (!WaterTowerStorage.layout().users[user].isAutoIrrigate) revert NotAutoIrrigate();
    _irrigate(user, rewardAmount);
    /// @dev 870391 is the gasLimit for this function
    uint256 gasFee = 870391 * tx.gasprice;
    WaterTowerStorage.layout().users[user].pending -= gasFee;
    emit AutoIrrigate(user, rewardAmount, gasFee);
}

As this function is called by the protocol admins, the gas fee paid by the admins is subtracted from the user pending balance. Although, the pending balance of the user will always be zero after an _irrigate() call of the full reward amount. Hence, if autoIrrigate() is called with the full reward amount the call will always revert.

RISK ACCEPTED: The Irrigation Protocol team accepted this risk, as they state that the autoIrrigate() function will always be called by an admin using full reward amount - fee amount.

The OwnershipFacet contract allows transferring the ownership of the IrrigationDiamond in a single step:

OwnershipFacet.sol

function transferOwnership(address _newOwner) external override {
    LibDiamond.enforceIsContractOwner();
    LibDiamond.setContractOwner(_newOwner);
}

If the nominated EOA account is not a valid account, it is entirely possible that the owner may accidentally transfer ownership to an uncontrolled account, losing the access to all functions with the onlySuperAdminRole modifier.

ACKNOWLEDGED: The Irrigation Protocol team acknowledged this finding.

The contract WaterTowerUpgradeable contains the function setMiddleAsset():

WaterTowerUpgradeable.sol

function setMiddleAsset(address middleAsset) external onlySuperAdminRole {
    WaterTowerStorage.layout().middleAssetForIrrigate = middleAsset;
}

If this function is ever called to set as middleAssetForIrrigate an asset different from the BEAN token, the irrigation bonus would always be zero as currently the _swapEthForWater() would only work if WaterTowerStorage.layout().middleAssetForIrrigate == Constants.BEAN:

WaterTowerUpgradeable.sol

function _swapEthForWater(uint256 amount) internal returns (uint256 waterAmount) {
    if (WaterTowerStorage.layout().middleAssetForIrrigate == Constants.BEAN) {
        /// @dev swap ETH for BEAN using curve router
        address[9] memory route = [
            Constants.ETHER,
            Constants.TRI_CRYPTO_POOL,
            Constants.USDT,
            Constants.CURVE_BEAN_METAPOOL,
            Constants.BEAN,
            Constants.ZERO,
            Constants.ZERO,
            Constants.ZERO,
            Constants.ZERO
        ];
        uint256[3][4] memory swapParams = [
            [uint(2), 0, 3],
            [uint(3), 0, 2],
            [uint(0), 0, 0],
            [uint(0), 0, 0]
        ];
        uint256 beanAmount = ICurveSwapRouter(Constants.CURVE_ROUTER).exchange_multiple{
            value: amount
        }(route, swapParams, amount, 0);

        waterAmount = ISprinklerUpgradeable(address(this)).getWaterAmount(
            Constants.BEAN,
            beanAmount
        );
    }
}

ACKNOWLEDGED: The Irrigation Protocol team acknowledged this finding.