Omniscia Steer Protocol Audit

BlackholePositionManagerFeeManager Manual Review Findings

BlackholePositionManagerFeeManager Manual Review Findings

BPM-01M: Unknown Library Integration

Description:

The referenced code block integrates with an unknown external library implementation in a sensitive capacity.

Impact:

This exhibit will be replaced with any items pertaining to the external library, if any.

Example:

src/BlackholePositionManagerFeeManager.sol
510// Use helper to figure out final amounts
511(shares, amount0Used, amount1Used) = IHelper(helper).getShares(
512    totalSupply(),
513    total0,
514    total1,
515    amount0Desired,
516    amount1Desired,
517    amount0Min,
518    amount1Min,
519    100 // MIN_SHARES
520);

Recommendation:

We advise the relevant library to be brought in scope, ensuring that its logic is validated.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The Steer Protocol team evaluated this exhibit and clarified that the underlying library has been audited in a separate engagement by our team.

As such, we consider this exhibit nullified.

BPM-02M: Partial Validation of Overlapping Ranges

Description:

The referenced conditional will partially validate overlapping ranges as it will still permit them when the new position's lowerTick sits between the previous position's lowerTick and upperTick values.

Impact:

The current code fails to properly prevent overlapping ranges and should either be removed to acknowledge this approach or corrected to prevent them properly.

Example:

src/BlackholePositionManagerFeeManager.sol
894if (_positions[i - 1].lowerTick >= _positions[i].lowerTick || _positions[i - 1].upperTick >= _positions[i].upperTick) revert PositionsOutOfOrder();

Recommendation:

We advise the code to be revised to ensure proper prohibition of overlapping ranges.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The Steer Protocol team evaluated this exhibit but opted to not apply it due to bytecode size constraints.

BPM-03M: Potentially Stale Data Point

Description:

The cache.currentTick value is not updated after a swap is performed rendering it outdated for the remainder of the function block.

Impact:

This exhibit highlights a potentially latent vulnerability in the BlackholePositionManager::tend function.

Example:

src/BlackholePositionManagerFeeManager.sol
716// 2. Load current pool state into `cache`
717(cache.sqrtPriceX96, cache.currentTick, , , , ) = pool.globalState();
718

719// 3. Protect against MEV / flashloan by verifying tick vs. TWAP
720// _doVolatilityCheck(cache.currentTick); -- @note skipped for this version
721

722// 4. Update global rewards and remove existing liquidity
723updateRewards();
724_burnAndCollect(1, 1);
725delete tokenIds;
726

727// There are 4 conditions of position and weight inputs:
728// -    defined positions,   totalWeight > 0    = Normal functionality
729// -    undefined positions, totalWeight > 0    = Previous positions are used
730// -    defined positions,   totalWeight = 0    = Empty positions, no liquidity deployed
731// -    undefined positions, totalWeight = 0    = Empty positions, no liquidity deployed
732

733// 5. Handle position migration and deletion based on conditions
734if (newPositions.lowerTick.length > 0 && totalWeight > 0) {
735    // Only migrate if new positions are provided AND will be used
736    migratePositions(newPositions);
737} else if (newPositions.lowerTick.length > 0 || totalWeight == 0) {
738    // Delete positions if new positions provided (but not used) OR totalWeight is 0
739    delete opPositions;
740}
741

742// 6. (Optional) Perform the swap
743_performSwap(cache);

Recommendation:

We advise the code to properly highlight that the cache.currentTick should not be utilized after the BlackholePositionManager::_performSwap function invocation.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

A comment has been introduced indicating that the cache.currentTick value should not be utilized beyond the volatility check in step 3 of the relevant function, rendering this exhibit to be addressed.

BPM-04M: Inexistent Re-Entrancy Protection

Description:

The BlackholePositionManager::updateRewards function is susceptible to re-entrancy attacks due to dynamically measuring balance deltas that can thus be resolved twice incorrectly through nested calls.

Impact:

Although exploitation would depend on non-standard reward tokens, the current BlackholePositionManager::updateRewards function is susceptible to re-entrancy attacks through duplicate accounting of the same balance delta.

Example:

src/BlackholePositionManagerFeeManager.sol
551function updateRewards() internal {
552       uint startingBalance = baseRewardToken.balanceOf(address(this));
553       uint startingBalanceExtra;
554       if (!_getFlag(6)) { // if skipExtra == false
555           startingBalanceExtra = extraRewardToken.balanceOf(address(this));
556       }
557       
558       IncentiveKey memory _key = incentiveKey;
559       uint netRegular;
560       uint netBonus;
561       
562       for (uint256 i = 0; i < tokenIds.length; i++) {
563           uint256 tokenId = tokenIds[i];
564           (uint additionalRegular, uint additionalBonus) = FARM_CENTER.collectRewards(_key, tokenId);
565           netRegular += additionalRegular;
566           netBonus += additionalBonus;
567       }
568

569       if (!_getFlag(6)) { // if skipExtra == false
570           FARM_CENTER.claimReward(baseRewardToken, address(this), netRegular);
571           FARM_CENTER.claimReward(extraRewardToken, address(this), netBonus);
572       }
573       else {
574           FARM_CENTER.claimReward(baseRewardToken, address(this), netRegular + netBonus);
575       }
576

577       uint256 extraFees;
578       uint256 totalSupply = totalSupply();
579

580       IFeeManager.Fee[] memory feesInfo = IFeeManager(feeManager).getFees(
581           address(this)
582       );
583       uint256 totalFees = IFeeManager(feeManager).vaultTotalFees(
584           address(this)
585       );
586

587       uint256 totalCutBase;
588       uint256 totalCutExtra;
589

590       if (!_getFlag(6)) { // if skipExtra == false
591           extraFees = extraRewardToken.balanceOf(address(this)) - startingBalanceExtra;
592           if (extraFees > 0) {
593               if (_getFlag(2)) { // rewardExtraIsPoolToken
594                   if (_getFlag(3)) { // rewardExtraIsToken0
595                       rewardBal0 += extraFees;
596                   } else {
597                       rewardBal1 += extraFees;
598                   }
599               }
600               totalCutExtra = FullMath.mulDiv(extraFees, totalFees, DIVISOR);
601               uint256 netGainExtra = extraFees - totalCutExtra;
602               reservedExtraFee += totalCutExtra;
603               accumulatedExtraRewardsPerShareP += ((netGainExtra * PRECISION)/
604                   totalSupply);
605               if (totalCutExtra > 0) {
606                   for (uint256 j; j != feesInfo.length; ++j) {
607                       accruedFees1[feesInfo[j].feeIdentifier] = accruedFees1[
608                           feesInfo[j].feeIdentifier
609                       ] + (FullMath.mulDiv(totalCutExtra, feesInfo[j].feeValue, DIVISOR));
610                   }
611               }
612           }
613       }
614

615       uint256 fees = baseRewardToken.balanceOf(address(this)) - startingBalance;
616       if (fees > 0) {
617           if (_getFlag(0)) { // rewardIsPoolToken
618               if (_getFlag(1)) { // rewardIsToken0
619                   rewardBal0 += fees;
620               } else {
621                   rewardBal1 += fees;
622               }
623           }
624           totalCutBase = FullMath.mulDiv(fees, totalFees, DIVISOR);
625           uint256 netGain = fees - totalCutBase;
626           accumulatedRewardsPerShareP += ((netGain * PRECISION) /
627               totalSupply);
628           reservedBaseFee += totalCutBase;
629           if (totalCutBase > 0 ) {
630               for (uint256 j; j != feesInfo.length; ++j) {
631                   accruedFees0[feesInfo[j].feeIdentifier] = accruedFees0[
632                       feesInfo[j].feeIdentifier
633                   ] + (FullMath.mulDiv(totalCutBase, feesInfo[j].feeValue, DIVISOR));
634               }
635           }
636       }
637

638       emit FeesEarned(fees, extraFees);
639   }

Recommendation:

We advise the code to prohibit re-entrancies through a re-entrancy guard, preventing the same balance increase from being counted twice.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The BlackholePositionManager::updateRewards function was updated to impose a re-entrancy guard, preventing any balance delta from being accounted for twice and thus alleviating this exhibit.

BPM-05M: Potential Truncation of Distributed Amounts

TypeSeverityLocation
Mathematical OperationsBlackholePositionManagerFeeManager.sol:
• I-1: L609
• I-2: L633

Description:

The totalCutExtra and totalCutBase values meant to be disbursed to eligible fee recipients by the BlackholePositionManager::updateRewards function will incur a truncation error on each fee recipient iteration resulting in fund loss.

Impact:

A miniscule amount of fees will bleed on each reward update of the BlackholePositionManager due to arithmetic truncation.

Example:

src/BlackholePositionManagerFeeManager.sol
606for (uint256 j; j != feesInfo.length; ++j) {
607    accruedFees1[feesInfo[j].feeIdentifier] = accruedFees1[
608        feesInfo[j].feeIdentifier
609    ] + (FullMath.mulDiv(totalCutExtra, feesInfo[j].feeValue, DIVISOR));
610}

Recommendation:

We advise each truncation to be accounted for by disbursing the final remaining amount to the last feeIdentifier.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The code was updated to properly account for truncations in both instances, evaluating all distributed cuts up to the final iteration and disbursing the remainder to the final recipient.

BPM-06M: Inexistent Access Control

Description:

The BlackholePositionManager::emergencyBurn function is available publicly, permitting any user to induce a Denial-of-Service on the BlackholePositionManager system.

Impact:

It is presently possible to forcibly claim rewards and burn positions from the AMM system.

Example:

src/BlackholePositionManagerFeeManager.sol
1161// Bailout for gas restrictive arrays, 
1162 //  ideally this is called for each tokenId in the positions array and then users can withdraw or fresh tend
1163 function emergencyBurn(uint tokenId, uint128 totalPosLiquidity) external returns (uint256 burned0, uint256 burned1) {
1164     (uint additionalRegular, uint additionalBonus) = FARM_CENTER.collectRewards(incentiveKey, tokenId);
1165     if (!_getFlag(6)) { // if skipExtra == false
1166         FARM_CENTER.claimReward(baseRewardToken, address(this), additionalRegular);
1167         FARM_CENTER.claimReward(extraRewardToken, address(this), additionalBonus);
1168     }
1169     else {
1170         FARM_CENTER.claimReward(baseRewardToken, address(this), additionalRegular + additionalBonus);
1171     }
1172 
1173     (burned0, burned1) = _burnAndCollectSingle(
1174         tokenId,
1175         totalPosLiquidity,
1176         1,
1177         1
1178     );
1179 }

Recommendation:

We advise proper access control to be imposed, ensuring that emergency functionality is solely accessible to authorized parties.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

Proper access control was introduced to the function by updating the BlackholePositionManager::emergencyBurn function to impose the STEER_ROLE restriction, addressing this exhibit.

BPM-07M: Insecure Tick Delta Evaluation

Description:

The referenced cumulative tick delta evaluation is insecure as it must be performed in an unchecked code block.

Impact:

Should the cumulative tick deltas overflow, the volatility check will consistently revert causing the system to undergo a Denial-of-Service.

Example:

src/BlackholePositionManagerFeeManager.sol
1215int56 tickCumulativesDelta = tickCumulatives[1] -
1216     tickCumulatives[0];

Recommendation:

We advise it to be done so, ensuring that volatility checks are safely imposed by the BlackholePositionManager.

Alleviation (bf4fc8ec2d63040ec62fb9ee839950cbf29a8b19):

The Steer Protocol team evaluated this exhibit in tandem with the Blackhole team and decided to omit the volatility check altogether due to a limitation in Blackhole's infrastructure, rendering this exhibit to have been alleviated as a result.

BPM-08M: Unauthorized Claim of Rewards

Description:

The BlackholePositionManager::transfer and BlackholePositionManager::transferFrom functions permit a user to claim rewards pending for another forcibly, causing automated smart contracts meant to integrate with the BlackholePositionManager to misbehave.

Impact:

Any smart contracts integrating with the BlackholePositionManager contract will fail to properly track reward tokens.

Example:

src/BlackholePositionManagerFeeManager.sol
1073function transfer(
1074        address to,
1075        uint256 value
1076    ) public override returns (bool) {
1077        address sender = msg.sender;
1078        updateRewards();
1079 
1080        pullRewards(sender);
1081        pullRewards(to);
1082 
1083        bool success = super.transfer(to, value);
1084 
1085        _updateUserDebts(sender);
1086        _updateUserDebts(to);
1087 
1088 
1089        return success;
1090    }

Recommendation:

We advise the code to not automatically pull rewards for recipients by instead storing them and permitting them to be claimed in a subsequent transaction.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

The code was updated to utilize a reward accrual and disbursement mechanism, permitting rewards to be accrued without being immediately claimed.

As such, we consider the original vulnerability properly alleviated.

BPM-09M: Inexistent Handling of Captured Rewards

Description:

The rewards collected by the BlackholePositionManager::emergencyBurn function are not handled, causing them to be permanently lost.

Impact:

Any rewards can be forcibly lost by combining the BlackholePositionManager::emergencyBurn function's access control absence and its mishandling of rewards.

Example:

src/BlackholePositionManagerFeeManager.sol
1163function emergencyBurn(uint tokenId, uint128 totalPosLiquidity) external returns (uint256 burned0, uint256 burned1) {
1164     (uint additionalRegular, uint additionalBonus) = FARM_CENTER.collectRewards(incentiveKey, tokenId);
1165     if (!_getFlag(6)) { // if skipExtra == false
1166         FARM_CENTER.claimReward(baseRewardToken, address(this), additionalRegular);
1167         FARM_CENTER.claimReward(extraRewardToken, address(this), additionalBonus);
1168     }
1169     else {
1170         FARM_CENTER.claimReward(baseRewardToken, address(this), additionalRegular + additionalBonus);
1171     }
1172 
1173     (burned0, burned1) = _burnAndCollectSingle(
1174         tokenId,
1175         totalPosLiquidity,
1176         1,
1177         1
1178     );
1179 }

Recommendation:

We advise them to be handled by either disbursing them to users or transferring them externally.

Alleviation (aafd869b2be09d232f2bbb4bdcbb3479d829f6ec):

A reward claim was introduced after the emergency burn operation, ensuring rewards are properly disbursed toward users after the burn operation has been performed.

As a result, we consider rewards correctly distributed and the original vulnerability to have been addressed.

Prev

Code Style

Next

BlackholePositionManagerFeeManager.sol (BPM-C)

On this page