Maia DAO Ecosystem - Madalad's results

Efficient liquidity renting and management across chains with Curvenized Uniswap V3.

General Information

Platform: Code4rena

Start Date: 30/05/2023

Pot Size: $300,500 USDC

Total HM: 79

Participants: 101

Period: about 1 month

Judge: Trust

Total Solo HM: 36

Id: 242

League: ETH

Maia DAO Ecosystem

Findings Distribution

Researcher Performance

Rank: 46/101

Findings: 6

Award: $546.05

QA:
grade-b
Analysis:
grade-b

🌟 Selected for report: 2

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: Madalad

Also found by: IllIllI, MohammedRizwan, ihtishamsudo, jasonxiale

Labels

bug
2 (Med Risk)
primary issue
satisfactory
selected for report
sponsor confirmed
M-17

Awards

224.671 USDC - $224.67

External Links

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L394-L415

Vulnerability details

Impact

Talos strategy contracts all inherit logic from TalosBaseStrategy, including the function collectProtocolFees. This function is used by the owner to receive fees earned by the contract.

Talos vault contracts should be expected to work properly for any token that has a sufficiently liquid Uniswap pool. However certain ERC20 tokens do not revert on failed transfer, and instead return false. In TalosBaseStrategy#collectProtocolFees, tokens are transferred from the contract to the owner using transfer, and the return value is not checked. This means that the transfer could fail silently, in which case protocolFees0 and protocolFees1 would be updated without the tokens leaving the contract. This function is inherited by any Talos vault contract.

This accounting discrepancy causes the tokens to be irretrievably trapped in the contract.

Proof of Concept

    function collectProtocolFees(uint256 amount0, uint256 amount1) external nonReentrant onlyOwner {
        uint256 _protocolFees0 = protocolFees0;
        uint256 _protocolFees1 = protocolFees1;

        if (amount0 > _protocolFees0) {
            revert Token0AmountIsBiggerThanProtocolFees();
        }
        if (amount1 > _protocolFees1) {
            revert Token1AmountIsBiggerThanProtocolFees();
        }
        ERC20 _token0 = token0;
        ERC20 _token1 = token1;
        uint256 balance0 = _token0.balanceOf(address(this));
        uint256 balance1 = _token1.balanceOf(address(this));
        require(balance0 >= amount0 && balance1 >= amount1);
        if (amount0 > 0) _token0.transfer(msg.sender, amount0); // @audit should use `safeTransfer`
        if (amount1 > 0) _token1.transfer(msg.sender, amount1); // @audit should use `safeTransfer`

        protocolFees0 = _protocolFees0 - amount0;
        protocolFees1 = _protocolFees1 - amount1;
        emit RewardPaid(msg.sender, amount0, amount1);
    }

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L394-L415

Tools Used

Manual review

Use OpenZeppelin's SafeERC20 library for ERC20 transfers.

Assessed type

ERC20

#0 - c4-judge

2023-07-09T14:27:03Z

trust1995 marked the issue as duplicate of #518

#1 - c4-judge

2023-07-11T08:29:24Z

trust1995 marked the issue as satisfactory

#2 - c4-judge

2023-07-25T13:16:44Z

trust1995 marked the issue as selected for report

#3 - deadrosesxyz

2023-07-26T21:33:04Z

Since we are talking about ERC20 transfer, the only reason for an erc20 transfer to fail would be insufficient balance. However, there is a require statement, checking if the balance is enough. This check makes silent fail impossible to happen.

#4 - trust1995

2023-07-27T09:40:14Z

Disagree, ERC20s are free to implement their own logic and transfer can fail for other reasons, e.g. blacklisted address. Therefore, using safeTransfer is a requirement.

#5 - c4-sponsor

2023-07-28T13:15:53Z

0xLightt marked the issue as sponsor confirmed

Awards

13.5257 USDC - $13.53

Labels

bug
2 (Med Risk)
downgraded by judge
primary issue
satisfactory
selected for report
sponsor confirmed
M-18

External Links

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L201-L210 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolActions.sol#L73-L87 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L135-L149 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L353-L361

Vulnerability details

Talos strategy contracts interact with Uniswap V3 in multiple areas of the code. However none of these interactions contain any slippage control, meaning that the contract, and by extension all users who hold shares, can lose significant value due to illiquid pools or MEV sandwich attacks every time any of the relevant functions are called.

Impact

TalosBaseStrategy#deposit is the entry point for any Talos vault, and transfers tokens from the caller to the vault to be deposited into Uniswap V3. Since it lacks slippage control, every user who interacts with any Talos vault will risk having their funds stolen by MEV bots. PoolActions#rerange is also vulnerable (which is called whenever the strategy manager wishes to rebalance pool allocation of the vault) which may lead to vault funds being at risk to the detriment of shareholders. The vault initialize function TalosBaseStrategy#init is vulnerable as well however only the vault owners funds would be at risk here.

Proof of Concept

In each of the below instances, a call to Uniswap V3 is made and amount0Min and amount1Min are each set to 0, which allows for 100% slippage tolerance. This means that the action could lead to the caller losing up to 100% of their tokens due to slippage.

TalosBaseStrategy#deposit:

        (liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity(
            INonfungiblePositionManager.IncreaseLiquidityParams({
                tokenId: _tokenId,
                amount0Desired: amount0Desired,
                amount1Desired: amount1Desired,
                amount0Min: 0, // @audit should be non-zero
                amount1Min: 0, // @audit should be non-zero
                deadline: block.timestamp
            })
        );

PoolActions#rerange:

        (tokenId, liquidity, amount0, amount1) = nonfungiblePositionManager.mint(
            INonfungiblePositionManager.MintParams({
                token0: address(actionParams.token0),
                token1: address(actionParams.token1),
                fee: poolFee,
                tickLower: tickLower,
                tickUpper: tickUpper,
                amount0Desired: balance0,
                amount1Desired: balance1,
                amount0Min: 0, // @audit should be non-zero
                amount1Min: 0, // @audit should be non-zero
                recipient: address(this),
                deadline: block.timestamp
            })

TalosBaseStrategy#init:

        (_tokenId, _liquidity, amount0, amount1) = _nonfungiblePositionManager.mint(
            INonfungiblePositionManager.MintParams({
                token0: address(_token0),
                token1: address(_token1),
                fee: poolFee,
                tickLower: tickLower,
                tickUpper: tickUpper,
                amount0Desired: amount0Desired,
                amount1Desired: amount1Desired,
                amount0Min: 0, // @audit should be non-zero
                amount1Min: 0, // @audit should be non-zero
                recipient: address(this),
                deadline: block.timestamp
            })
        );

TalosBaseStrategy#_withdrawAll:

        _nonfungiblePositionManager.decreaseLiquidity(
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: _tokenId,
                liquidity: _liquidity,
                amount0Min: 0, // @audit should be non-zero
                amount1Min: 0, // @audit should be non-zero
                deadline: block.timestamp
            })
        );

Tools Used

Manual review

For each vulnerable function, allow the caller to specify values for amount0Min and amount1Min instead of setting them to 0.

Assessed type

Uniswap

#0 - c4-judge

2023-07-11T08:24:46Z

trust1995 marked the issue as duplicate of #177

#1 - c4-judge

2023-07-11T08:24:51Z

trust1995 marked the issue as satisfactory

#2 - c4-judge

2023-07-11T17:04:10Z

trust1995 changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-07-11T17:04:20Z

trust1995 changed the severity to 3 (High Risk)

#4 - c4-judge

2023-07-25T08:54:03Z

trust1995 changed the severity to 2 (Med Risk)

#5 - c4-judge

2023-07-25T13:46:08Z

trust1995 marked the issue as selected for report

#6 - 0xLightt

2023-07-26T14:32:31Z

none of these interactions contain any slippage control

Just want to add that this is not accurate. These functions, except init, already have the checkDeviation modifier that offers some level of protection.

But this issue is still valid, since the pool's slippage protection offered by the modifier may not be the same as the desired by the user. This way, the user can define his own settings.

#7 - c4-sponsor

2023-07-28T13:15:58Z

0xLightt marked the issue as sponsor confirmed

Findings Information

Labels

bug
2 (Med Risk)
satisfactory
duplicate-534

Awards

23.9127 USDC - $23.91

External Links

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L342

Vulnerability details

In UniswapV3Staker, endIncentive can be called to end an incentive after its endTime, and refund unclaimed rewards to the minter address. However, it requires that the number of stakes for that incentive is 0. To account for users who do not unstake their own tokens, the restakeToken function exists to allow arbitrary callers to unstake any tokenId and restake it in a new incentive, provided that block.timestamp is after the endTime of the incentive that that tokenId is currently staked in. However, due to incorrect logic in restakeToken, users are unable to restake anyone elses tokenId.

Impact

If a tokenId remains staked after the end of an epoch, then endIncentive will always revert. This can occur intentionally by a malicious user staking an NFT representing a Uniswap position with negligible value, or unintentionally should a user forget about their stake or lose access to their private key. As the functionality to unstake other users NFTs through restake does not work, this results in a permanent DoS of the endIncentive function, which is used to refund any excess reward tokens to the minter address. This means that those funds would be trapped in the contract indefinitely.

Proof of Concept

UniswapV3Staker#_unstakeToken has a boolean parameter isNotRestake which is meant to be false if the unstaking process is part of a restake, and true otherwise:

File: src/uni-v3-staker/UniswapV3Staker.sol

365:    function _unstakeToken(IncentiveKey memory key, uint256 tokenId, bool isNotRestake) private {

However when _unstake is called within restake, the value for isNotRestake is passed as true, when it should be false:

File: src/uni-v3-staker/UniswapV3Staker.sol

340:    function restakeToken(uint256 tokenId) external {
341:        IncentiveKey storage incentiveId = stakedIncentiveKey[tokenId];
342:        if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true); // @audit should pass `false` here
343:
344:        (IUniswapV3Pool pool, int24 tickLower, int24 tickUpper, uint128 liquidity) =
345:            NFTPositionInfo.getPositionInfo(factory, nonfungiblePositionManager, tokenId);
346:
347:        _stakeToken(tokenId, pool, tickLower, tickUpper, liquidity);
348:    }

As a result, the check in _unstakeToken on line 374 will always revert if the caller is not the owner. Since isNotRestake is always true (restakeToken, unstakeToken), the expression within the if statement is identical to

(true || block.timestamp < endTime) && owner != msg.sender <=> true && owner != msg.sender <=> owner != msg.sender
File: src/uni-v3-staker/UniswapV3Staker.sol

365:    function _unstakeToken(IncentiveKey memory key, uint256 tokenId, bool isNotRestake) private {
366:        Deposit storage deposit = deposits[tokenId];
367:
368:        (uint96 endTime, uint256 stakedDuration) =
369:            IncentiveTime.getEndAndDuration(key.startTime, deposit.stakedTimestamp, block.timestamp);
370:
371:        address owner = deposit.owner;
372:
373:        // anyone can call restakeToken if the block time is after the end time of the incentive
374:        if ((isNotRestake || block.timestamp < endTime) && owner != msg.sender) revert NotCalledByOwner(); // @audit isNotRestake is always true

            // @audit rest of function omitted for readability

Therefore, only the owner can unstake a tokenId, even if restake was called and block.timestamp exceeds the epoch endTime.

Tools Used

Manual review

Change restakeToken to pass the correct isNotRestake parameter:

    function restakeToken(uint256 tokenId) external {
        IncentiveKey storage incentiveId = stakedIncentiveKey[tokenId];
-       if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true);
+       if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, false);

        (IUniswapV3Pool pool, int24 tickLower, int24 tickUpper, uint128 liquidity) =
            NFTPositionInfo.getPositionInfo(factory, nonfungiblePositionManager, tokenId);

        _stakeToken(tokenId, pool, tickLower, tickUpper, liquidity);
    }

Assessed type

DoS

#0 - c4-judge

2023-07-11T08:28:49Z

trust1995 marked the issue as duplicate of #745

#1 - c4-judge

2023-07-11T08:28:54Z

trust1995 marked the issue as satisfactory

Awards

14.356 USDC - $14.36

Labels

bug
2 (Med Risk)
satisfactory
duplicate-504

External Links

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L201-L210 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L263-L271 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolActions.sol#L73-L87 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L135-L149 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L353-L361

Vulnerability details

Impact

Uniswap docs state that:

Transaction `deadline` is used to set a time after which a transaction can no longer be executed. This limits the "free option" problem, where Ethereum miners can hold signed transactions and execute them based off market movements.

Talos strategy contracts interact with Uniswap V3 in multiple areas of the code. However in each case, deadline is set to block.timestamp. This effectively means there is no deadline, and that the transaction can be executed arbitrarily far into the future, when market conditions may have changed such that the action is now largely unprofitable. This opens up the vault to be exploited through MEV every time it interacts with Uni V3, putting users funds at risk. See here for a similar issue from another C4 contest.

Proof of Concept

In each of the below instances, a call to Uniswap V3 is made and deadline is set to block.timestamp. This means that the action could be executed far into the future, by which time the caller (strategy manager or depositor) may not wish to execute such an action anymore, due to market conditions, pool liquidity or any other factors.

TalosBaseStrategy#deposit:

        (liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity(
            INonfungiblePositionManager.IncreaseLiquidityParams({
                tokenId: _tokenId,
                amount0Desired: amount0Desired,
                amount1Desired: amount1Desired,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp // @audit no deadline set
            })
        );

TalosBaseStrategy#redeem:

            (amount0, amount1) = _nonfungiblePositionManager.decreaseLiquidity(
                INonfungiblePositionManager.DecreaseLiquidityParams({
                    tokenId: _tokenId,
                    liquidity: liquidityToDecrease,
                    amount0Min: amount0Min,
                    amount1Min: amount1Min,
                    deadline: block.timestamp
                })
            );

PoolActions#rerange:

        (tokenId, liquidity, amount0, amount1) = nonfungiblePositionManager.mint(
            INonfungiblePositionManager.MintParams({
                token0: address(actionParams.token0),
                token1: address(actionParams.token1),
                fee: poolFee,
                tickLower: tickLower,
                tickUpper: tickUpper,
                amount0Desired: balance0,
                amount1Desired: balance1,
                amount0Min: 0,
                amount1Min: 0,
                recipient: address(this),
                deadline: block.timestamp // @audit no deadline set
            })

TalosBaseStrategy#init:

        (_tokenId, _liquidity, amount0, amount1) = _nonfungiblePositionManager.mint(
            INonfungiblePositionManager.MintParams({
                token0: address(_token0),
                token1: address(_token1),
                fee: poolFee,
                tickLower: tickLower,
                tickUpper: tickUpper,
                amount0Desired: amount0Desired,
                amount1Desired: amount1Desired,
                amount0Min: 0,
                amount1Min: 0,
                recipient: address(this),
                deadline: block.timestamp // @audit no deadline set
            })
        );

TalosBaseStrategy#_withdrawAll:

        _nonfungiblePositionManager.decreaseLiquidity(
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: _tokenId,
                liquidity: _liquidity,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp // @audit no deadline set
            })
        );

Tools Used

Manual review

For each vulnerable function, allow the caller to specify a value for deadline instead of setting it to block.timestamp.

Assessed type

Uniswap

#0 - c4-judge

2023-07-09T11:18:02Z

trust1995 marked the issue as duplicate of #171

#1 - c4-judge

2023-07-09T11:18:06Z

trust1995 marked the issue as satisfactory

Awards

74.2737 USDC - $74.27

Labels

bug
grade-b
QA (Quality Assurance)
Q-04

External Links

Summary

Low Risk

IssueInstances
[L-01]BaseV2GaugeFactory#gaugeId default value can point to different, active gauge1
[L-02]Consider adding sweep functionality for NFTs sent to UniswapV3Staker using ERC721#transfer1
[L-03]Must approve 0 first2
[L-04]Solidity version is vulnerable to assembly bug20
[L-05]Consider adding denylist9
[L-06]Missing contract existence check for low level call10
[L-07]decimals() is not part of the ERC20 standard20
[L-08]Division before multiplications leads to precision loss1
[L-09]Checking bytecode length is insufficient to check user is an EOA2
[L-10]Empty receive/payable fallback function3
[L-11]External call recipient may consume all transaction gas10
[L-12]Use fixed compiler version58
[L-13]initialize can be frontrun2
[L-14]CEI pattern not followed23
[L-15]Only one of _mint, _safeMint implemented2
[L-16]Use safeTransfer instead of transfer for token transfers2
[L-17]Setters should emit events12
[L-18]Solidity version <=0.8.14 is susceptible to an optimiser bug30
[L-19]Solidity version 0.8.20 may not work on other chains due to PUSH085
[L-20]Handle case when totalSupply is 02
[L-21]Downcasting uint or int may result in overflow18
[L-22]Unused Ownable2
[L-23]Missing address(0) checks in constructor/initialize61

Total issues: 21

Total instances: 374

 

Low Risk Issues

[L-01] BaseV2GaugeFactory#gaugeId default value can point to different, active gauge

Push null gauge to zero index like done in UlyssesPool's constructor to prevent nonexistent gauges from pointing to the gauge at index 1.

    constructor(
        uint256 _id,
        address _asset,
        string memory _name,
        string memory _symbol,
        address _owner,
        address _factory
    ) UlyssesERC4626(_asset, _name, _symbol) {
        require(_owner != address(0));
        factory = UlyssesFactory(_factory);
        _initializeOwner(_owner);
        require(_id != 0);
        id = _id;

        bandwidthStateList.push(BandwidthState({bandwidth: 0, destination: UlyssesPool(address(0)), weight: 0}));
    }

Instances: 1

https://github.com/code-423n4/2023-05-maia/blob/main/src/gauges/factories/BaseV2GaugeFactory.sol#L36

[L-02] Consider adding sweep functionality for NFTs sent to UniswapV3Staker using ERC721#transfer

The fact that users must directly send their NFT to the UniswapV3Staker contract rather than calling a deposit function introduces the possibility that they use ERC721#transfer rather than ERC721#safeTransfer, which would not trigger the onERC721Received function and lead to the NFT being stuck in the contract. Implementing a sweep function would allow a trusted admin to return NFTs mistakenly sent in this way.

e.g.

function sweepToken(uint256 tokenId, address receiver) external onlyOwner {
    require(nonfungiblePositionManager.ownerOf(tokenId) == address(this), "invalid tokenId");
    require(deposits[tokenId].owner == address(0), "tokenId was deposited");

    nonfungiblePositionManager.safeTransfer(receiver, tokenId);
}

Instances: 1

https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L218-L236

[L-03] Must approve 0 first

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

Recommendation: add an approval with amount 0 before approving the desired amount, or use safeApprove from OpenZeppelin's SafeERC20 library.

<details><summary>Instances: 2</summary>
File: src/ulysses-omnichain/MulticallRootRouter.sol

120:         ERC20hTokenRoot(outputToken).approve(bridgeAgentAddress, amountOut);

148:             ERC20hTokenRoot(outputTokens[i]).approve(bridgeAgentAddress, amountsOut[i]);

L120, L148

</details>

 

[L-04] Solidity version is vulnerable to assembly bug

A bug introduced in Solidity version 0.8.13 and patched in version 0.8.17 incorrectly removes storage writes within assembly blocks. See here for more information.

<details><summary>Instances: 20</summary>
File: src/governance/GovernorBravoDelegateMaia.sol

538:         assembly {
539:             chainId := chainid()
540:         }

L538

File: src/ulysses-amm/UlyssesPool.sol

380:         assembly {
381:             // Store bandwidth state slot in memory
382:             mstore(0x00, bandwidthStateList.slot)
383:             // Hash the bandwidth state slot to get the bandwidth state list start
384:             let bandwidthStateListStart := keccak256(0x00, 0x20)

386:             // Total difference from target bandwidth of all bandwidth states
387:             let totalDiff
388:             // Total difference from target bandwidth of all bandwidth states
389:             let transfered
390:             // Total amount to be distributed according to each bandwidth weights
391:             let transferedChange

393:             for { let i := 1 } lt(i, length) { i := add(i, 1) } {
394:                 // Load bandwidth and weight from storage
395:                 // Each bandwidth state occupies two storage slots
396:                 let slot := sload(add(bandwidthStateListStart, mul(i, 2)))
397:                 // Bandwidth is the first 248 bits of the slot
398:                 let bandwidth := and(slot, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
399:                 // Weight is the last 8 bits of the slot
400:                 let weight := shr(248, slot)

402:                 // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
403:                 if mul(weight, gt(_totalSupply, div(not(0), weight))) {
404:                     // Store the function selector of `MulDivFailed()`.
405:                     mstore(0x00, 0xad251c27)
406:                     // Revert with (offset, size).
407:                     revert(0x1c, 0x04)
408:                 }

410:                 // Calculate the target bandwidth
411:                 let targetBandwidth := div(mul(_totalSupply, weight), _totalWeights)

413:                 // Calculate the difference from the target bandwidth
414:                 switch positiveTransfer
415:                 // If the transfer is positive, calculate deficit from target bandwidth
416:                 case true {
417:                     // If there is a deficit, store the difference
418:                     if gt(targetBandwidth, bandwidth) {
419:                         // Calculate the difference
420:                         let diff := sub(targetBandwidth, bandwidth)
421:                         // Add the difference to the total difference
422:                         totalDiff := add(totalDiff, diff)
423:                         // Store the difference in the diffs array
424:                         mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff)
425:                     }
426:                 }
427:                 // If the transfer is negative, calculate surplus from target bandwidth
428:                 default {
429:                     // If there is a surplus, store the difference
430:                     if gt(bandwidth, targetBandwidth) {
431:                         // Calculate the difference
432:                         let diff := sub(bandwidth, targetBandwidth)
433:                         // Add the difference to the total difference
434:                         totalDiff := add(totalDiff, diff)
435:                         // Store the difference in the diffs array
436:                         mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff)
437:                     }
438:                 }
439:             }

441:             // Calculate the amount to be distributed according deficit/surplus
442:             // and/or the amount to be distributed according to each bandwidth weights
443:             switch gt(amount, totalDiff)
444:             // If the amount is greater than the total deficit/surplus
445:             case true {
446:                 // Total deficit/surplus is distributed
447:                 transfered := totalDiff
448:                 // Set rest to be distributed according to each bandwidth weights
449:                 transferedChange := sub(amount, totalDiff)
450:             }
451:             // If the amount is less than the total deficit/surplus
452:             default {
453:                 // Amount will be distributed according to deficit/surplus
454:                 transfered := amount
455:             }

457:             for { let i := 1 } lt(i, length) { i := add(i, 1) } {
458:                 // Increase/decrease amount of bandwidth for each bandwidth state
459:                 let bandwidthUpdate

461:                 // If there is a deficit/surplus, calculate the amount to be distributed
462:                 if gt(transfered, 0) {
463:                     // Load the difference from the diffs array
464:                     let diff := mload(add(diffs, add(mul(i, 0x20), 0x20)))

466:                     // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
467:                     if mul(diff, gt(transfered, div(not(0), diff))) {
468:                         // Store the function selector of `MulDivFailed()`.
469:                         mstore(0x00, 0xad251c27)
470:                         // Revert with (offset, size).
471:                         revert(0x1c, 0x04)
472:                     }

474:                     // Calculate the amount to be distributed according to deficit/surplus
475:                     switch roundUp
476:                     // If round up then do mulDivUp(transfered, diff, totalDiff)
477:                     case true {
478:                         bandwidthUpdate :=
479:                             add(
480:                                 iszero(iszero(mod(mul(transfered, diff), totalDiff))), div(mul(transfered, diff), totalDiff)
481:                             )
482:                     }
483:                     // If round down then do mulDiv(transfered, diff, totalDiff)
484:                     default { bandwidthUpdate := div(mul(transfered, diff), totalDiff) }
485:                 }

487:                 // If there is a rest, calculate the amount to be distributed according to each bandwidth weights
488:                 if gt(transferedChange, 0) {
489:                     // Load weight from storage
490:                     let weight := shr(248, sload(add(bandwidthStateListStart, mul(i, 2))))

492:                     // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
493:                     if mul(weight, gt(transferedChange, div(not(0), weight))) {
494:                         // Store the function selector of `MulDivFailed()`.
495:                         mstore(0x00, 0xad251c27)
496:                         // Revert with (offset, size).
497:                         revert(0x1c, 0x04)
498:                     }

500:                     // Calculate the amount to be distributed according to each bandwidth weights
501:                     switch roundUp
502:                     // If round up then do mulDivUp(transferedChange, weight, _totalWeights)
503:                     case true {
504:                         bandwidthUpdate :=
505:                             add(
506:                                 bandwidthUpdate,
507:                                 add(
508:                                     iszero(iszero(mod(mul(transferedChange, weight), _totalWeights))),
509:                                     div(mul(transferedChange, weight), _totalWeights)
510:                                 )
511:                             )
512:                     }
513:                     // If round down then do mulDiv(transferedChange, weight, _totalWeights)
514:                     default {
515:                         bandwidthUpdate := add(bandwidthUpdate, div(mul(transferedChange, weight), _totalWeights))
516:                     }
517:                 }

519:                 // If there is an update in bandwidth
520:                 if gt(bandwidthUpdate, 0) {
521:                     // Store the amount to be updated in the bandwidthUpdateAmounts array
522:                     mstore(add(bandwidthUpdateAmounts, add(mul(i, 0x20), 0x20)), bandwidthUpdate)
523:                 }
524:             }
525:         }

553:         assembly {
554:             // Load bandwidth and weight from storage
555:             let slot := sload(destinationState.slot)
556:             // Bandwidth is the first 248 bits of the slot
557:             bandwidth := and(slot, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
558:             // Weight is the last 8 bits of the slot
559:             weight := shr(248, slot)

561:             // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
562:             if mul(weight, gt(_totalSupply, div(not(0), weight))) {
563:                 // Store the function selector of `MulDivFailed()`.
564:                 mstore(0x00, 0xad251c27)
565:                 // Revert with (offset, size).
566:                 revert(0x1c, 0x04)
567:             }

569:             // Get the target bandwidth
570:             targetBandwidth := div(mul(_totalSupply, weight), _totalWeights)
571:         }

581:         assembly {
582:             switch positiveTransfer
583:             // If the transfer is positive
584:             case true {
585:                 // Add the difference to the bandwidth
586:                 bandwidth := add(bandwidth, difference)

588:                 // Revert if bandwidth overflows
589:                 if lt(bandwidth, difference) {
590:                     // Store the function selector of `Overflow()`.
591:                     mstore(0x00, 0x35278d12)
592:                     // Revert with (offset, size).
593:                     revert(0x1c, 0x04)
594:                 }
595:             }
596:             // If the transfer is negative
597:             default {
598:                 // Revert if bandwidth underflows
599:                 if gt(difference, bandwidth) {
600:                     // Store the function selector of `Underflow()`.
601:                     mstore(0x00, 0xcaccb6d9)
602:                     // Revert with (offset, size).
603:                     revert(0x1c, 0x04)
604:                 }

606:                 // Subtract the difference from the bandwidth
607:                 bandwidth := sub(bandwidth, difference)
608:             }

610:             // True on deposit, mint and redeem
611:             if gt(_newTotalSupply, 0) {
612:                 // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
613:                 if mul(weight, gt(_newTotalSupply, div(not(0), weight))) {
614:                     // Store the function selector of `MulDivFailed()`.
615:                     mstore(0x00, 0xad251c27)
616:                     // Revert with (offset, size).
617:                     revert(0x1c, 0x04)
618:                 }

620:                 // Get the new target bandwidth after total supply change
621:                 targetBandwidth := div(mul(_newTotalSupply, weight), _totalWeights)
622:             }
623:         }

633:         assembly {
634:             switch lt(newRebalancingFee, oldRebalancingFee)
635:             // If new fee is lower than old fee
636:             case true {
637:                 // Calculate the positive fee
638:                 positivefee := sub(oldRebalancingFee, newRebalancingFee)

640:                 // If depositFees is true, add the positive fee to the bandwidth
641:                 if depositFees {
642:                     bandwidth := add(bandwidth, positivefee)

644:                     // Revert if bandwidth overflows
645:                     if lt(bandwidth, positivefee) {
646:                         // Store the function selector of `Overflow()`.
647:                         mstore(0x00, 0x35278d12)
648:                         // Revert with (offset, size).
649:                         revert(0x1c, 0x04)
650:                     }
651:                 }
652:             }
653:             default {
654:                 // If new fee is higher than old fee
655:                 if gt(newRebalancingFee, oldRebalancingFee) {
656:                     // Calculate the negative fee
657:                     negativeFee := sub(newRebalancingFee, oldRebalancingFee)
658:                 }
659:             }

661:             if gt(bandwidth, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) {
662:                 // Store the function selector of `Overflow()`.
663:                 mstore(0x00, 0x35278d12)
664:                 // Revert with (offset, size).
665:                 revert(0x1c, 0x04)
666:             }

668:             // Update storage with the new bandwidth
669:             sstore(destinationState.slot, or(bandwidth, shl(248, weight)))
670:         }

698:         assembly {
699:             // Load the rebalancing fee slot to get the fee parameters
700:             let feeSlot := sload(fees.slot)
701:             // Get sigma2 from the first 8 bytes of the fee slot
702:             let sigma2 := shr(192, feeSlot)
703:             // Get sigma1 from the next 8 bytes of the fee slot
704:             let sigma1 := and(shr(128, feeSlot), 0xffffffffffffffff)
705:             // Get lambda2 from the next 8 bytes of the fee slot
706:             lambda2 := and(shr(64, feeSlot), 0xffffffffffffffff)
707:             // Get lambda1 from the last 8 bytes of the fee slot
708:             lambda1 := and(feeSlot, 0xffffffffffffffff)

710:             // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y))
711:             if mul(sigma1, gt(targetBandwidth, div(not(0), sigma1))) {
712:                 // Store the function selector of `MulDivFailed()`.
713:                 mstore(0x00, 0xad251c27)
714:                 // Revert with (offset, size).
715:                 revert(0x1c, 0x04)
716:             }

718:             // Calculate the upper bound for the first fee
719:             upperBound1 := div(mul(targetBandwidth, sigma1), DIVISIONER)

721:             // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y))
722:             if mul(sigma2, gt(targetBandwidth, div(not(0), sigma2))) {
723:                 // Store the function selector of `MulDivFailed()`.
724:                 mstore(0x00, 0xad251c27)
725:                 // Revert with (offset, size).
726:                 revert(0x1c, 0x04)
727:             }

729:             // Calculate the upper bound for the second fee
730:             upperBound2 := div(mul(targetBandwidth, sigma2), DIVISIONER)
731:         }

737:         assembly {
738:             // Calculate the maximum width of the trapezium
739:             maxWidth := sub(upperBound1, upperBound2)
740:         }

751:             assembly {
752:                 // offset = lambda1 * 2
753:                 lambda1 := shl(1, lambda1)
754:             }

760:             assembly {
761:                 // Add the two fees together
762:                 fee := add(fee, fee2)
763:             }

851:         assembly {
852:             // Calculate the height of the trapezium
853:             // The height is calculated as `upperBound - bandwidth`
854:             let height := sub(upperBound, bandwidth)

856:             // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
857:             if mul(feeTier, gt(height, div(not(0), feeTier))) {
858:                 // Store the function selector of `MulDivFailed()`.
859:                 mstore(0x00, 0xad251c27)
860:                 // Revert with (offset, size).
861:                 revert(0x1c, 0x04)
862:             }

864:             // Calculate the width of the trapezium, rounded up
865:             // The width is calculated as `feeTier * height / maxWidth + offset`
866:             let width :=
867:                 add(add(iszero(iszero(mod(mul(height, feeTier), maxWidth))), div(mul(height, feeTier), maxWidth)), offset)

869:             // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y))
870:             if mul(height, gt(width, div(not(0), height))) {
871:                 // Store the function selector of `MulDivFailed()`.
872:                 mstore(0x00, 0xad251c27)
873:                 // Revert with (offset, size).
874:                 revert(0x1c, 0x04)
875:             }

877:             // Calculate the fee for this tier
878:             switch roundDown
879:             // If round down then do mulDiv(transfered, diff, totalDiff)
880:             case true { fee := div(mul(width, height), DIVISIONER) }
881:             // If round up then do mulDivUp(transfered, diff, totalDiff)
882:             default {
883:                 fee := add(iszero(iszero(mod(mul(width, height), DIVISIONER))), div(mul(width, height), DIVISIONER))
884:             }
885:         }

913:                 assembly {
914:                     // Add updateAmount to output
915:                     output := add(output, updateAmount)
916:                 }

948:         assembly {
949:             // Get the new total supply by adding amount to totalSupply
950:             _newTotalSupply := add(_totalSupply, amount)
951:         }

1007:         assembly {
1008:             // Revert if output underflows
1009:             if gt(negativeFee, output) {
1010:                 // Store the function selector of `Underflow()`.
1011:                 mstore(0x00, 0xcaccb6d9)
1012:                 // Revert with (offset, size).
1013:                 revert(0x1c, 0x04)
1014:             }

1016:             // Subtracting assets to deposit or shares to mint
1017:             output := sub(output, negativeFee)
1018:         }

1032:         assembly {
1033:             // Get the old total supply by adding burned shares to totalSupply
1034:             _totalSupply := add(_newTotalSupply, shares)
1035:         }

1050:                 assembly {
1051:                     // Add updateAmount to assets
1052:                     assets := add(assets, updateAmount)
1053:                 }

1062:                 assembly {
1063:                     // Update negativeFee
1064:                     negativeFee := add(negativeFee, _negativeFee)
1065:                 }

1074:         assembly {
1075:             // Revert if assets underflows
1076:             if gt(negativeFee, assets) {
1077:                 // Store the function selector of `Underflow()`.
1078:                 mstore(0x00, 0xcaccb6d9)
1079:                 // Revert with (offset, size).
1080:                 revert(0x1c, 0x04)
1081:             }

1083:             // Subtracting assets to withdraw
1084:             assets := sub(assets, negativeFee)
1085:         }

1112:         assembly {
1113:             // Get the protocol fee from storage
1114:             let _protocolFee := sload(protocolFee.slot)

1116:             // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y))
1117:             if mul(_protocolFee, gt(assets, div(not(0), _protocolFee))) {
1118:                 // Store the function selector of `MulDivFailed()`.
1119:                 mstore(0x00, 0xad251c27)
1120:                 // Revert with (offset, size).
1121:                 revert(0x1c, 0x04)
1122:             }

1124:             // Calculate the base fee, rounding up
1125:             let baseFee :=
1126:                 add(iszero(iszero(mod(mul(assets, _protocolFee), DIVISIONER))), div(mul(assets, _protocolFee), DIVISIONER))

1128:             // Revert if assets underflows
1129:             if gt(baseFee, assets) {
1130:                 // Store the function selector of `Underflow()`.
1131:                 mstore(0x00, 0xcaccb6d9)
1132:                 // Revert with (offset, size).
1133:                 revert(0x1c, 0x04)
1134:             }

1136:             // Subtract the base fee from assets
1137:             output := sub(assets, baseFee)
1138:         }

1175:         assembly {
1176:             // Revert if output underflows
1177:             if gt(negativeFee, assets) {
1178:                 // Store the function selector of `Underflow()`.
1179:                 mstore(0x00, 0xcaccb6d9)
1180:                 // Revert with (offset, size).
1181:                 revert(0x1c, 0x04)
1182:             }

1184:             // Subtract the negative fee from assets
1185:             output := sub(assets, negativeFee)
1186:         }

L380, L386, L393, L402, L410, L413, L441, L457, L461, L466, L474, L487, L492, L500, L519, L553, L561, L569, L581, L588, L606, L610, L620, L633, L640, L644, L661, L668, L698, L710, L718, L721, L729, L737, L751, L760, L851, L856, L864, L869, L877, L913, L948, L1007, L1016, L1032, L1050, L1062, L1074, L1083, L1112, L1116, L1124, L1128, L1136, L1175, L1184

File: src/maia/libraries/DateTimeLib.sol

43:         assembly {
44:             epochDay := add(epochDay, 719468)
45:             let doe := mod(epochDay, 146097)
46:             let yoe := div(sub(sub(add(doe, div(doe, 36524)), div(doe, 1460)), eq(doe, 146096)), 365)
47:             let doy := sub(doe, sub(add(mul(365, yoe), shr(2, yoe)), div(yoe, 100)))
48:             let mp := div(add(mul(5, doy), 2), 153)
49:             month := sub(add(mp, 3), mul(gt(mp, 9), 12))
50:         }

L43

</details>

 

[L-05] Consider adding denylist

A denylist helps to prevent malicious users from spending stolen ERC20 or ERC721 tokens in the protocol.

<details><summary>Instances: 9</summary>
File: src/ulysses-omnichain/VirtualAccount.sol

36:     function withdrawERC721(address _token, uint256 _tokenId) external requiresApprovedCaller {

60:     function onERC721Received(address, address, uint256, bytes calldata) external pure override returns (bytes4) {

L36, L60

File: src/hermes/bHermes.sol

158:     function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {

L158

File: src/talos/TalosStrategyStaked.sol

94:     function transferFrom(address _from, address _to, uint256 _amount) public override returns (bool) {

L94

File: src/maia/tokens/ERC4626PartnerManager.sol

279:     function transferFrom(address from, address to, uint256 amount)
280:         public
281:         virtual
282:         override
283:         checkTransfer(from, amount)
284:         returns (bool)
285:     {

L279

File: src/erc-20/ERC20Boost.sol

323:     function transferFrom(address from, address to, uint256 amount)
324:         public
325:         override
326:         notAttached(from, amount)
327:         returns (bool)
328:     {

L323

File: src/erc-20/ERC20MultiVotes.sol

303:     function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {

L303

File: src/talos/base/TalosBaseStrategy.sol

328:     function onERC721Received(address, address, uint256, bytes calldata) external pure override returns (bytes4) {

L328

File: src/erc-20/ERC20Gauges.sol

508:     function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {

L508

</details>

 

[L-06] Missing contract existence check for low level call

Low-level calls return success if there is no code present at the specified address. Add a check to verify that target.code.size > 0 (or in assembly, use extcodesize).

<details><summary>Instances: 10</summary>
File: src/ulysses-omnichain/VirtualAccount.sol

49:             (bool success, bytes memory data) = calls[i].target.call(calls[i].callData);

L49

File: src/ulysses-omnichain/MulticallRootRouter.sol

281:             IVirtualAccount(userAccount).call(calls);

289:             IVirtualAccount(userAccount).call(calls);

309:             IVirtualAccount(userAccount).call(calls);

357:             IVirtualAccount(userAccount).call(calls);

365:             IVirtualAccount(userAccount).call(calls);

385:             IVirtualAccount(userAccount).call(calls);

433:             IVirtualAccount(userAccount).call(calls);

441:             IVirtualAccount(userAccount).call(calls);

461:             IVirtualAccount(userAccount).call(calls);

L281, L289, L309, L357, L365, L385, L433, L441, L461

</details>

 

[L-07] decimals() is not part of the ERC20 standard

The decimals() function is not a part of the ERC-20 standard, and was addedlater as an optional extension. As such, some valid ERC20 tokens do not support this interface, so it is unsafe to blindly cast all tokens to this interface, and then call this function.

<details><summary>Instances: 20</summary>
File: src/ulysses-amm/UlyssesToken.sol

46:         require(ERC20(asset).decimals() == 18);

L46

File: src/ulysses-omnichain/ArbitrumBranchPort.sol

72:         underlyingAddress.safeTransfer(_recipient, _denormalizeDecimals(_deposit, ERC20(underlyingAddress).decimals()));

81:         _underlyingAddress.safeTransfer(
82:             _recipient, _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals())
83:         );

119:             _underlyingAddress.safeTransferFrom(
120:                 _depositor, address(this), _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals())
121:             );

138:                 _underlyingAddresses[i].safeTransferFrom(
139:                     _depositor,
140:                     address(this),
141:                     _denormalizeDecimals(_deposits[i], ERC20(_underlyingAddresses[i]).decimals())
142:                 );

L72, L81, L119, L138

File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol

103:         IArbPort(localPortAddress).depositToPort(
104:             msg.sender, msg.sender, underlyingAddress, _normalizeDecimals(amount, ERC20(underlyingAddress).decimals())
105:         );

L103

File: src/ulysses-omnichain/BranchPort.sol

211:         _underlyingAddress.safeTransfer(
212:             _recipient, _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals())
213:         );

253:             _underlyingAddress.safeTransferFrom(
254:                 _depositor, address(this), _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals())
255:             );

269:                 _underlyingAddresses[i].safeTransferFrom(
270:                     _depositor,
271:                     address(this),
272:                     _denormalizeDecimals(_deposits[i], ERC20(_underlyingAddresses[i]).decimals())
273:                 );

L211, L253, L269

File: src/ulysses-omnichain/BranchBridgeAgent.sol

245:         bytes memory packedData = abi.encodePacked(
246:             bytes1(0x05),
247:             msg.sender,
248:             depositNonce,
249:             _dParams.hToken,
250:             _dParams.token,
251:             _dParams.amount,
252:             _normalizeDecimals(_dParams.deposit, ERC20(_dParams.token).decimals()),
253:             _dParams.toChain,
254:             _params,
255:             msg.value.toUint128(),
256:             _remoteExecutionGas
257:         );

284:             _deposits[i] = _normalizeDecimals(_dParams.deposits[i], ERC20(_dParams.tokens[i]).decimals());

334:                 packedData = abi.encodePacked(
335:                     bytes1(0x05),
336:                     msg.sender,
337:                     _depositNonce,
338:                     getDeposit[_depositNonce].hTokens[0],
339:                     getDeposit[_depositNonce].tokens[0],
340:                     getDeposit[_depositNonce].amounts[0],
341:                     _normalizeDecimals(
342:                         getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals()
343:                     ),
344:                     _toChain,
345:                     _params,
346:                     msg.value.toUint128(),
347:                     _remoteExecutionGas
348:                 );

350:                 packedData = abi.encodePacked(
351:                     bytes1(0x02),
352:                     _depositNonce,
353:                     getDeposit[_depositNonce].hTokens[0],
354:                     getDeposit[_depositNonce].tokens[0],
355:                     getDeposit[_depositNonce].amounts[0],
356:                     _normalizeDecimals(
357:                         getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals()
358:                     ),
359:                     _toChain,
360:                     _params,
361:                     msg.value.toUint128(),
362:                     _remoteExecutionGas
363:                 );

681:         bytes memory packedData = abi.encodePacked(
682:             bytes1(0x02),
683:             depositNonce,
684:             _dParams.hToken,
685:             _dParams.token,
686:             _dParams.amount,
687:             _normalizeDecimals(_dParams.deposit, ERC20(_dParams.token).decimals()),
688:             _dParams.toChain,
689:             _params,
690:             _gasToBridgeOut,
691:             _remoteExecutionGas
692:         );

720:             deposits[i] = _normalizeDecimals(_dParams.deposits[i], ERC20(_dParams.tokens[i]).decimals());

1355:             deposits[i] = _normalizeDecimals(_deposits[i], ERC20(_tokens[i]).decimals());

L245, L284, L334, L350, L681, L720, L1355

File: src/erc-4626/ERC4626DepositOnly.sol

23:     constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {

L23

File: src/erc-4626/UlyssesERC4626.sol

27:         if (ERC20(_asset).decimals() != 18) revert InvalidAssetDecimals();

L27

File: src/erc-4626/ERC4626.sol

23:     constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {

L23

File: src/erc-4626/ERC4626MultiToken.sol

51:             require(ERC20(_assets[i]).decimals() == 18);

L51

</details>

 

[L-08] Division before multiplications leads to precision loss

Since Solidity cannot deal with decimal values, make sure to use multiplication before division to avoid precision loss due to rounding errors.

<details><summary>Instances: 1</summary>
File: src/uni-v3-staker/libraries/RewardMath.sol

34:             boostedSecondsInsideX128 = uint160(
35:                 ((secondsInsideX128 * 4) / 10) + ((((stakedDuration << 128) * boostAmount) / boostTotalSupply) * 6) / 10
36:             );

L34

</details>

 

[L-09] Checking bytecode length is insufficient to check user is an EOA

The protocol implements an EOA check by ensuring that the bytecode length at the address is equal to 0. However this can be gamed since during a smart contracts constructor, its bytecode is technically empty. Therefore the check can be passed by a smart contract. While unlikely, this edge case is still worth consideration.

<details><summary>Instances: 2</summary>
File: src/erc-20/ERC20MultiVotes.sol

105:         if (canExceedMax && account.code.length == 0) revert Errors.NonContractError(); // can only approve contracts

L105

File: src/erc-20/ERC20Gauges.sol

464:         if (canExceedMax && account.code.length == 0) revert Errors.NonContractError(); // can only approve contracts

L464

</details>

 

[L-10] Empty receive/payable fallback function

If the intention is for the ether to be used, the function should call another function or emit an event, otherwise it should revert (e.g. require(msg.sender == address(weth))), otherwise users might send ether to the contract mistakenly and lose funds.

<details><summary>Instances: 3</summary>
File: src/ulysses-omnichain/CoreBranchRouter.sol

284:     fallback() external payable {}

L284

File: src/ulysses-omnichain/RootBridgeAgent.sol

1334:     fallback() external payable {}

L1334

File: src/ulysses-omnichain/BranchBridgeAgent.sol

1419:     fallback() external payable {}

L1419

</details>

 

[L-11] External call recipient may consume all transaction gas

There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert. Use addr.call{gas: <amount>}("") or this library instead.

<details><summary>Instances: 10</summary>
File: src/ulysses-omnichain/VirtualAccount.sol

49:             (bool success, bytes memory data) = calls[i].target.call(calls[i].callData);

L49

File: src/ulysses-omnichain/MulticallRootRouter.sol

281:             IVirtualAccount(userAccount).call(calls);

289:             IVirtualAccount(userAccount).call(calls);

309:             IVirtualAccount(userAccount).call(calls);

357:             IVirtualAccount(userAccount).call(calls);

365:             IVirtualAccount(userAccount).call(calls);

385:             IVirtualAccount(userAccount).call(calls);

433:             IVirtualAccount(userAccount).call(calls);

441:             IVirtualAccount(userAccount).call(calls);

461:             IVirtualAccount(userAccount).call(calls);

L281, L289, L309, L357, L365, L385, L433, L441, L461

</details>

 

[L-12] Use fixed compiler version

A floating pragma should not be used for non-interface contracts as it risks a different compiler version being used in production vs testing, which may lead to unintended and unexpected discrepencies between the behaviour of the protocol during testing and during production.

<details><summary>Instances: 58</summary>
File: src/hermes/tokens/HERMES.sol

2: pragma solidity ^0.8.0;

L2

File: src/maia/tokens/Maia.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/booster/FlywheelBoosterGaugeWeight.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/depots/SingleRewardsDepot.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesBoost.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/rewards/FlywheelBribeRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/rewards/FlywheelInstantRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/FlywheelCoreInstant.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/FlywheelCoreStrategy.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesGauges.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesVotes.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/TalosStrategyVanillaFactory.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/factories/BoostAggregatorFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/OptimizerFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/UniswapV3Gauge.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/depots/MultiRewardsDepot.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/token/ERC20hTokenRoot.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/VirtualAccount.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/factories/ERC20hTokenRootFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/TalosStrategyStakedFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/maia/factories/PartnerManagerFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-amm/UlyssesRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/RootBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosOptimizer.sol

3: pragma solidity ^0.8.0;

L3

File: src/governance/GovernorBravoDelegator.sol

2: pragma solidity ^0.8.10;

L2

File: src/maia/vMaia.sol

3: pragma solidity ^0.8.0;

L3

File: src/gauges/factories/BribesFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/factories/UniswapV3GaugeFactory.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/TalosManager.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/ArbitrumBranchBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/bHermes.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-amm/UlyssesToken.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchBridgeAgentExecutor.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/factories/BaseV2GaugeManager.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/factories/BranchBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/minters/BaseV2Minter.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/boost-aggregator/BoostAggregator.sol

2: pragma solidity >=0.8.0;

L2

File: src/ulysses-omnichain/BaseBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosStrategyVanilla.sol

3: pragma solidity >=0.8.0;

L3

File: src/ulysses-amm/factories/UlyssesFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/ArbitrumBranchPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosStrategyStaked.sol

2: pragma solidity >=0.8.0;

L2

File: src/governance/GovernorBravoInterfaces.sol

2: pragma solidity ^0.8.10;

L2

File: src/rewards/rewards/FlywheelGaugeRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/CoreBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/CoreRootRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/MulticallRootRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/RootPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/uni-v3-staker/UniswapV3Staker.sol

3: pragma solidity ^0.8.0;

L3

File: src/governance/GovernorBravoDelegateMaia.sol

2: pragma solidity ^0.8.10;

L2

File: src/ulysses-amm/UlyssesPool.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/RootBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

</details>

 

[L-13] initialize can be frontrun

Lack of access control on initialize function means that it can be frontrun, allowing a malicious user to steal ownership of the contract and necessitating an expensive re-deployment.

<details><summary>Instances: 2</summary>
File: src/hermes/minters/BaseV2Minter.sol

78:     function initialize(FlywheelGaugeRewards _flywheelGaugeRewards) external {

L78

File: src/talos/base/TalosBaseStrategy.sol

102:     function init(uint256 amount0Desired, uint256 amount1Desired, address receiver)
103:         external
104:         virtual
105:         nonReentrant
106:         returns (uint256 shares, uint256 amount0, uint256 amount1)
107:     {

L102

</details>

 

[L-14] CEI pattern not followed

The Checks-Effects-Interactions pattern is a best practice that reduces the attack surface for reentrancy attacks.

Place state variable updates before external calls within functions.

<details><summary>Instances: 23</summary>
File: src/talos/boost-aggregator/BoostAggregator.sol

159:     function withdrawProtocolFees(address to) external onlyOwner {
160:         uniswapV3Staker.claimReward(to, protocolRewards);
161:         delete protocolRewards;

L159

File: src/talos/TalosStrategyStaked.sol

164:     function _unstake(uint256 _tokenId) internal {

166:         boostAggregator.unstakeAndWithdraw(_tokenId);

168:         stakeFlag = false;

173:     function _stake(uint256 _tokenId) internal {

177:         try nonfungiblePositionManager.safeTransferFrom(address(this), address(boostAggregator), _tokenId) {
178:             stakeFlag = true; // flag to store staking state to avoid failing to unstake when it is not staked

L164, L166, L168, L173, L177

File: src/rewards/rewards/FlywheelGaugeRewards.sol

72:     function queueRewardsForCycle() external returns (uint256 totalQueuedForCycle) {

76:         minter.updatePeriod();

85:         gaugeCycle = currentCycle;

100:         nextCycleQueuedRewards = 0;
101:         paginationOffset = 0;

107:     function queueRewardsForCyclePaginated(uint256 numRewards) external {

111:         minter.updatePeriod();

121:             nextCycle = currentCycle;
122:             paginationOffset = 0;

144:             gaugeCycle = currentCycle;
145:             nextCycleQueuedRewards = 0;
146:             paginationOffset = 0;

149:             paginationOffset = offset + numRewards.toUint32();

L72, L76, L85, L100, L107, L111, L121, L144, L149

File: src/ulysses-omnichain/RootPort.sol

140:     function initializeCore(
141:         address _coreRootBridgeAgent,
142:         address _coreLocalBranchBridgeAgent,
143:         address _localBranchPortAddress
144:     ) external onlyOwner {

153:         IBridgeAgent(_coreRootBridgeAgent).syncBranchBridgeAgent(_coreLocalBranchBridgeAgent, localChainId);
154:         getBridgeAgentManager[_coreRootBridgeAgent] = owner();

420:     function addNewChain(
421:         address _pledger,
422:         uint256 _pledgedInitialAmount,
423:         address _coreBranchBridgeAgentAddress,
424:         uint24 _chainId,
425:         string memory _wrappedGasTokenName,
426:         string memory _wrappedGasTokenSymbol,
427:         uint24 _fee,
428:         uint24 _priceImpactPercentage,
429:         uint160 _sqrtPriceX96,
430:         address _nonFungiblePositionManagerAddress,
431:         address _newLocalBranchWrappedNativeTokenAddress,
432:         address _newUnderlyingBranchWrappedNativeTokenAddress
433:     ) external onlyOwner {

440:         ERC20hTokenRoot(newGlobalToken).mint(_pledger, _pledgedInitialAmount, _chainId);

442:         IBridgeAgent(ICoreRootRouter(coreRootRouterAddress).bridgeAgentAddress()).syncBranchBridgeAgent(
443:             _coreBranchBridgeAgentAddress, _chainId
444:         );

446:         getWrappedNativeToken[_chainId] = _newUnderlyingBranchWrappedNativeTokenAddress;
447:         isChainId[_chainId] = true;
448:         isGlobalAddress[newGlobalToken] = true;
449:         getGlobalTokenFromLocal[_newLocalBranchWrappedNativeTokenAddress][_chainId] = newGlobalToken;
450:         getLocalTokenFromGlobal[newGlobalToken][_chainId] = _newLocalBranchWrappedNativeTokenAddress;
451:         getLocalTokenFromUnder[_newUnderlyingBranchWrappedNativeTokenAddress][_chainId] =
452:             _newLocalBranchWrappedNativeTokenAddress;
453:         getUnderlyingTokenFromLocal[_newLocalBranchWrappedNativeTokenAddress][_chainId] =
454:             _newUnderlyingBranchWrappedNativeTokenAddress;

473:         getGasPoolInfo[chainId] = GasPoolInfo({
474:             zeroForOneOnInflow: zeroForOneOnInflow,
475:             priceImpactPercentage: _priceImpactPercentage,
476:             gasTokenGlobalAddress: newGlobalToken,
477:             poolAddress: newGasPoolAddress
478:         });

L140, L153, L420, L440, L442, L446, L473

File: src/uni-v3-staker/UniswapV3Staker.sol

476:     function _stakeToken(uint256 tokenId, IUniswapV3Pool pool, int24 tickLower, int24 tickUpper, uint128 liquidity)
477:         private
478:     {

496:         if (!hermesGaugeBoost.isUserGauge(tokenOwner, address(gauge))) {
497:             _userAttachements[tokenOwner][pool] = tokenId;

507:             _stakes[tokenId][incentiveId] = Stake({
508:                 secondsPerLiquidityInsideInitialX128: secondsPerLiquidityInsideX128,
509:                 liquidityNoOverflow: type(uint96).max,
510:                 liquidityIfOverflow: liquidity
511:             });

L476, L496, L507

File: src/ulysses-amm/UlyssesPool.sol

159:     function addNewBandwidth(uint256 poolId, uint8 weight) external nonReentrant onlyOwner returns (uint256 index) {

202:         bandwidthStateList.push(
203:             BandwidthState({bandwidth: newBandwidth.toUint248(), destination: destination, weight: weight})
204:         );

206:         destinations[destinationId] = index;
207:         destinationIds[address(destination)] = index;

L159, L202, L206

File: src/ulysses-omnichain/RootBridgeAgent.sol

860:     function anyExecute(bytes calldata data)
861:         external
862:         virtual
863:         requiresExecutor
864:         returns (bool success, bytes memory result)
865:     {

926:             try RootBridgeAgentExecutor(bridgeAgentExecutorAddress).executeSystemRequest(
927:                 localRouterAddress, data, fromChainId
928:             ) returns (bool, bytes memory res) {

936:             executionHistory[fromChainId][nonce] = true;

961:             executionHistory[fromChainId][nonce] = true;

985:             executionHistory[fromChainId][nonce] = true;

1009:             executionHistory[fromChainId][nonce] = true;

1045:             executionHistory[fromChainId][nonce] = true;

1080:             executionHistory[fromChainId][nonce] = true;

1115:             executionHistory[fromChainId][nonce] = true;

1138:             executionHistory[fromChainId][nonce] = true;

1148:                 executionHistory[fromChainId][nonce] = true;

L860, L926, L936, L961, L985, L1009, L1045, L1080, L1115, L1138, L1148

File: src/ulysses-omnichain/BranchBridgeAgent.sol

857:     function _createDepositSingle(
858:         address _user,
859:         address _hToken,
860:         address _token,
861:         uint256 _amount,
862:         uint256 _deposit,
863:         uint128 _gasToBridgeOut
864:     ) internal {

866:         IPort(localPortAddress).bridgeOut(_user, _hToken, _token, _amount, _deposit);

882:         getDeposit[_getAndIncrementDepositNonce()] = Deposit({
883:             owner: _user,
884:             hTokens: hTokens,
885:             tokens: tokens,
886:             amounts: amounts,
887:             deposits: deposits,
888:             status: DepositStatus.Success,
889:             depositedGas: _gasToBridgeOut
890:         });

903:     function _createDepositMultiple(
904:         address _user,
905:         address[] memory _hTokens,
906:         address[] memory _tokens,
907:         uint256[] memory _amounts,
908:         uint256[] memory _deposits,
909:         uint128 _gasToBridgeOut
910:     ) internal {

912:         IPort(localPortAddress).bridgeOutMultiple(_user, _hTokens, _tokens, _amounts, _deposits);

918:         getDeposit[_getAndIncrementDepositNonce()] = Deposit({
919:             owner: _user,
920:             hTokens: _hTokens,
921:             tokens: _tokens,
922:             amounts: _amounts,
923:             deposits: _deposits,
924:             status: DepositStatus.Success,
925:             depositedGas: _gasToBridgeOut
926:         });

1118:     function anyExecute(bytes calldata data)
1119:         external
1120:         virtual
1121:         requiresExecutor
1122:         returns (bool success, bytes memory result)
1123:     {

1154:             try BranchBridgeAgentExecutor(bridgeAgentExecutorAddress).executeNoSettlement(localRouterAddress, data)
1155:             returns (bool, bytes memory res) {

1162:             executionHistory[nonce] = true;

1186:             executionHistory[nonce] = true;

1210:             executionHistory[nonce] = true;

L857, L866, L882, L903, L912, L918, L1118, L1154, L1162, L1186, L1210

File: src/talos/factories/TalosBaseStrategyFactory.sol

55:     function createTalosBaseStrategy(
56:         IUniswapV3Pool pool,
57:         ITalosOptimizer optimizer,
58:         address strategyManager,
59:         bytes memory data
60:     ) external {
61:         if (optimizerFactory.optimizerIds(TalosOptimizer(address(optimizer))) == 0) {

67:         strategyIds[strategy] = strategies.length;

L55, L67

File: src/gauges/BaseV2Gauge.sol

128:     function addBribeFlywheel(FlywheelCore bribeFlywheel) external onlyOwner {

133:         FlywheelBribeRewards(flyWheelRewards).setRewardsDepot(multiRewardsDepot);

135:         multiRewardsDepot.addAsset(flyWheelRewards, bribeFlywheel.rewardToken()); // @audit not removed ever ?

137:         isActive[bribeFlywheel] = true;
138:         added[bribeFlywheel] = true;

L128, L133, L135, L137

File: src/rewards/base/FlywheelCore.sol

125:     function setFlywheelRewards(address newFlywheelRewards) external onlyOwner {

128:             rewardToken.safeTransferFrom(address(flywheelRewards), address(newFlywheelRewards), oldRewardBalance);

131:         flywheelRewards = newFlywheelRewards;

L125, L128, L131

File: src/maia/tokens/ERC4626PartnerManager.sol

216:     function increaseConversionRate(uint256 newRate) external onlyOwner {

219:         if (newRate > (address(bHermesToken).balanceOf(address(this)) / totalSupply)) {

223:         bHermesRate = newRate;

L216, L219, L223

File: src/erc-20/ERC20Boost.sol

116:     function attach(address user) external {
117:         if (!_gauges.contains(msg.sender) || _deprecatedGauges.contains(msg.sender)) {

127:             getUserBoost[user] = userGaugeBoost;

150:     function updateUserBoost(address user) external {

159:             if (!_deprecatedGauges.contains(gauge)) {

169:         getUserBoost[user] = userBoost;

L116, L127, L150, L159, L169

File: src/talos/base/TalosBaseStrategy.sol

102:     function init(uint256 amount0Desired, uint256 amount1Desired, address receiver)
103:         external
104:         virtual
105:         nonReentrant
106:         returns (uint256 shares, uint256 amount0, uint256 amount1)
107:     {

119:             PoolVariables.checkRange(_tickLower, _tickUpper); // Check ticks also for overflow/underflow
120:             tickLower = _tickLower;
121:             tickUpper = _tickUpper;

154:         liquidity = _liquidity;
155:         tokenId = _tokenId;

163:         initialized = true;

349:     function _withdrawAll(uint256 _tokenId) internal {

353:         _nonfungiblePositionManager.decreaseLiquidity(
354:             INonfungiblePositionManager.DecreaseLiquidityParams({
355:                 tokenId: _tokenId,
356:                 liquidity: _liquidity,
357:                 amount0Min: 0,
358:                 amount1Min: 0,
359:                 deadline: block.timestamp
360:             })
361:         );
362:         liquidity = 0;

394:     function collectProtocolFees(uint256 amount0, uint256 amount1) external nonReentrant onlyOwner {

409:         if (amount0 > 0) _token0.transfer(msg.sender, amount0);
410:         if (amount1 > 0) _token1.transfer(msg.sender, amount1);

412:         protocolFees0 = _protocolFees0 - amount0;
413:         protocolFees1 = _protocolFees1 - amount1;

L102, L119, L154, L163, L349, L353, L394, L409, L412

File: src/erc-20/ERC20Gauges.sol

292:     function _decrementGaugeWeight(address user, address gauge, uint112 weight, uint32 cycle) internal {

297:         IBaseV2Gauge(gauge).accrueBribes(user);

299:         getUserGaugeWeight[user][gauge] = oldWeight - weight;

L292, L297, L299

</details>

 

[L-15] Only one of _mint, _safeMint implemented

If one of the functions is re-implemented, or has new arguments, the other should be as well. The _mint variant is supposed to skip onERC721Received checks, whereas _safeMint does not. Not having both points to a possible issue with spec-compatability.

<details><summary>Instances: 2</summary>
File: src/hermes/bHermes.sol

128:     function _mint(address to, uint256 amount) internal virtual override {

L128

File: src/maia/tokens/ERC4626PartnerManager.sol

240:     function _mint(address to, uint256 amount) internal virtual override {

L240

</details>

 

[L-16] Use safeTransfer instead of transfer for token transfers

SafeERC20 is a wrapper around ERC20 operations that throw on failure (when the token contract returns false). This prevents calls executing if a token transfer is unsuccessful. Simply add using SafeERC20 for ERC20.

<details><summary>Instances: 2</summary>
File: src/talos/base/TalosBaseStrategy.sol

409:         if (amount0 > 0) _token0.transfer(msg.sender, amount0);

410:         if (amount1 > 0) _token1.transfer(msg.sender, amount1);

L409, L410

</details>

 

[L-17] Setters should emit events

Permissioned setter functions without a timelock should emit events so that users can react to critical protocol changes.

<details><summary>Instances: 12</summary>
File: src/talos/TalosOptimizer.sol

62:     function setMaxTotalSupply(uint256 _maxTotalSupply) external onlyOwner {

68:     function setTwapDuration(uint32 _twapDuration) external onlyOwner {

74:     function setMaxTwapDeviation(int24 _maxTwapDeviation) external onlyOwner {

80:     function setTickRange(int24 _tickRangeMultiplier) external onlyOwner {

85:     function setPriceImpact(uint24 _priceImpactPercentage) external onlyOwner {

L62, L68, L74, L80, L85

File: src/gauges/factories/UniswapV3GaugeFactory.sol

98:     function setMinimumWidth(address gauge, uint24 minimumWidth) external onlyOwner {

L98

File: src/hermes/minters/BaseV2Minter.sol

86:     function setDao(address _dao) external onlyOwner {

92:     function setDaoShare(uint256 _daoShare) external onlyOwner {

98:     function setTailEmission(uint256 _tail_emission) external onlyOwner {

L86, L92, L98

File: src/talos/boost-aggregator/BoostAggregator.sol

153:     function setProtocolFee(uint256 _protocolFee) external onlyOwner {

L153

File: src/ulysses-amm/UlyssesPool.sol

223:     function setWeight(uint256 poolId, uint8 weight) external nonReentrant onlyOwner {

308:     function setFees(Fees calldata _fees) external nonReentrant onlyOwner {

L223, L308

</details>

 

[L-18] Solidity version <=0.8.14 is susceptible to an optimiser bug

In solidity versions 0.8.13 and 0.8.14, there is an optimizer bug where, if the use of a variable is in a separate assembly block from the block in which it was stored, the mstore operation is optimized out, leading to uninitialized memory. The code currently does not have such a pattern of execution, but it does use mstores in assembly blocks, so it is a risk for future changes. The affected solidity versions should be avoided if at all possible.

See https://medium.com/certora/overly-optimistic-optimizer-certora-bug-disclosure-2101e3f7994d for more details.

<details><summary>Instances: 30</summary>
File: src/governance/GovernorBravoDelegator.sol

2: pragma solidity ^0.8.10;

76:             let free_mem_ptr := mload(0x40)
77:             returndatacopy(free_mem_ptr, 0, returndatasize())

L2, L76

File: src/ulysses-amm/UlyssesPool.sol

2: pragma solidity ^0.8.0;

360:                 mstore(0x00, 0x87138d5c)

368:                 mstore(0x00, 0xc2f5625a)

382:             mstore(0x00, bandwidthStateList.slot)

405:                     mstore(0x00, 0xad251c27)

424:                         mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff)

436:                         mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff)

464:                     let diff := mload(add(diffs, add(mul(i, 0x20), 0x20)))

469:                         mstore(0x00, 0xad251c27)

495:                         mstore(0x00, 0xad251c27)

522:                     mstore(add(bandwidthUpdateAmounts, add(mul(i, 0x20), 0x20)), bandwidthUpdate)

564:                 mstore(0x00, 0xad251c27)

591:                     mstore(0x00, 0x35278d12)

601:                     mstore(0x00, 0xcaccb6d9)

615:                     mstore(0x00, 0xad251c27)

647:                         mstore(0x00, 0x35278d12)

663:                 mstore(0x00, 0x35278d12)

713:                 mstore(0x00, 0xad251c27)

724:                 mstore(0x00, 0xad251c27)

859:                 mstore(0x00, 0xad251c27)

872:                 mstore(0x00, 0xad251c27)

1011:                 mstore(0x00, 0xcaccb6d9)

1078:                 mstore(0x00, 0xcaccb6d9)

1102:                 mstore(0x00, 0x3c930918)

1119:                 mstore(0x00, 0xad251c27)

1131:                 mstore(0x00, 0xcaccb6d9)

1156:                 mstore(0x00, 0x3c930918)

1163:                 mstore(0x00, 0xc2f5625a)

1179:                 mstore(0x00, 0xcaccb6d9)

L2, L360, L368, L382, L405, L424, L436, L464, L469, L495, L522, L564, L591, L601, L615, L647, L663, L713, L724, L859, L872, L1011, L1078, L1102, L1119, L1131, L1156, L1163, L1179

</details>

 

[L-19] Solidity version 0.8.20 may not work on other chains due to PUSH0

The compiler for Solidity 0.8.20 switches the default target EVM version to Shanghai, which includes the new PUSH0 op code. This op code may not yet be implemented on all L2s, so deployment on these chains will fail. See this relevant issue on the official Solidity github for reference. To work around this issue, use an earlier EVM version.

<details><summary>Instances: 85</summary>
File: src/hermes/tokens/HERMES.sol

2: pragma solidity ^0.8.0;

L2

File: src/maia/tokens/Maia.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/booster/FlywheelBoosterGaugeWeight.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/depots/SingleRewardsDepot.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesBoost.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/rewards/FlywheelBribeRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/rewards/FlywheelInstantRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/FlywheelCoreInstant.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/FlywheelCoreStrategy.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesGauges.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/tokens/bHermesVotes.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/TalosStrategyVanillaFactory.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/factories/BoostAggregatorFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/OptimizerFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/UniswapV3Gauge.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/depots/MultiRewardsDepot.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/token/ERC20hTokenRoot.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/VirtualAccount.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/factories/ERC20hTokenRootFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/factories/TalosStrategyStakedFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/maia/factories/PartnerManagerFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-amm/UlyssesRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/RootBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosOptimizer.sol

3: pragma solidity ^0.8.0;

L3

File: src/governance/GovernorBravoDelegator.sol

2: pragma solidity ^0.8.10;

L2

File: src/maia/vMaia.sol

3: pragma solidity ^0.8.0;

L3

File: src/gauges/factories/BribesFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/factories/UniswapV3GaugeFactory.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/TalosManager.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/factories/ArbitrumBranchBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/bHermes.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-amm/UlyssesToken.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchBridgeAgentExecutor.sol

2: pragma solidity ^0.8.0;

L2

File: src/gauges/factories/BaseV2GaugeManager.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/factories/BranchBridgeAgentFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/minters/BaseV2Minter.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/boost-aggregator/BoostAggregator.sol

2: pragma solidity >=0.8.0;

L2

File: src/ulysses-omnichain/BaseBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosStrategyVanilla.sol

3: pragma solidity >=0.8.0;

L3

File: src/ulysses-amm/factories/UlyssesFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/ArbitrumBranchPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/TalosStrategyStaked.sol

2: pragma solidity >=0.8.0;

L2

File: src/governance/GovernorBravoInterfaces.sol

2: pragma solidity ^0.8.10;

L2

File: src/rewards/rewards/FlywheelGaugeRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/CoreBranchRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/CoreRootRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/MulticallRootRouter.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/RootPort.sol

3: pragma solidity ^0.8.0;

L3

File: src/uni-v3-staker/UniswapV3Staker.sol

3: pragma solidity ^0.8.0;

L3

File: src/governance/GovernorBravoDelegateMaia.sol

2: pragma solidity ^0.8.10;

L2

File: src/ulysses-amm/UlyssesPool.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/RootBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/BranchBridgeAgent.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/depots/RewardsDepot.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/base/BaseFlywheelRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/rewards/rewards/FlywheelAcummulatedRewards.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/strategies/TalosStrategySimple.sol

2: pragma solidity >=0.8.0;

L2

File: src/talos/factories/TalosBaseStrategyFactory.sol

2: pragma solidity ^0.8.0;

L2

File: src/erc-4626/ERC4626DepositOnly.sol

2: pragma solidity >=0.8.0;

L2

File: src/erc-4626/UlyssesERC4626.sol

2: pragma solidity >=0.8.0;

L2

File: src/gauges/BaseV2Gauge.sol

2: pragma solidity ^0.8.0;

L2

File: src/hermes/UtilityManager.sol

3: pragma solidity ^0.8.0;

L3

File: src/erc-4626/ERC4626.sol

2: pragma solidity >=0.8.0;

L2

File: src/gauges/factories/BaseV2GaugeFactory.sol

3: pragma solidity ^0.8.0;

L3

File: src/maia/PartnerUtilityManager.sol

2: pragma solidity ^0.8.0;

L2

File: src/rewards/base/FlywheelCore.sol

3: pragma solidity ^0.8.0;

L3

File: src/maia/tokens/ERC4626PartnerManager.sol

2: pragma solidity ^0.8.0;

L2

File: src/erc-4626/ERC4626MultiToken.sol

2: pragma solidity >=0.8.0;

L2

File: src/erc-20/ERC20Boost.sol

3: pragma solidity ^0.8.0;

L3

File: src/erc-20/ERC20MultiVotes.sol

4: pragma solidity ^0.8.0;

L4

File: src/talos/base/TalosBaseStrategy.sol

3: pragma solidity >=0.8.0;

L3

File: src/erc-20/ERC20Gauges.sol

3: pragma solidity ^0.8.0;

L3

File: src/uni-v3-staker/libraries/IncentiveId.sol

2: pragma solidity ^0.8.0;

L2

File: src/ulysses-omnichain/lib/AnycallFlags.sol

3: pragma solidity ^0.8.10;

L3

File: src/maia/libraries/DateTimeLib.sol

2: pragma solidity ^0.8.4;

L2

File: src/uni-v3-staker/libraries/NFTPositionInfo.sol

3: pragma solidity ^0.8.0;

L3

File: src/uni-v3-staker/libraries/IncentiveTime.sol

2: pragma solidity ^0.8.0;

L2

File: src/uni-v3-staker/libraries/RewardMath.sol

2: pragma solidity ^0.8.0;

L2

File: src/talos/libraries/PoolActions.sol

3: pragma solidity ^0.8.0;

L3

File: src/talos/libraries/PoolVariables.sol

3: pragma solidity ^0.8.0;

L3

</details>

 

[L-20] Handle case when totalSupply is 0

The case in which totalSupply is equal to zero should be explicitly handled to avoid unexpected behaviour.

<details><summary>Instances: 2</summary>
File: src/maia/tokens/ERC4626PartnerManager.sol

219:         if (newRate > (address(bHermesToken).balanceOf(address(this)) / totalSupply)) {

L219

File: src/talos/base/TalosBaseStrategy.sol

261:             uint128 liquidityToDecrease = uint128((liquidity * shares) / totalSupply);

L261

</details>

 

[L-21] Downcasting uint or int may result in overflow

Consider using OpenZeppelin's SafeCast library to prevent unexpected overflows.

<details><summary>Instances: 18</summary>
File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol

160:         DepositMultipleParams memory dParams = _bridgeInMultiple(
161:             _router,
162:             _data[
163:                 PARAMS_START:
164:                     PARAMS_END_OFFSET + uint16(uint8(bytes1(_data[PARAMS_START]))) * PARAMS_TKN_SET_SIZE_MULTIPLE
165:             ],
166:             _fromChainId
167:         );

177:             (success, result) = IRouter(_router).anyExecuteDepositMultiple(
178:                 bytes1(_data[PARAMS_END_OFFSET + uint16(numOfAssets) * PARAMS_TKN_SET_SIZE_MULTIPLE]),
179:                 _data[
180:                     PARAMS_START + PARAMS_END_OFFSET + uint16(numOfAssets) * PARAMS_TKN_SET_SIZE_MULTIPLE:
181:                         length - PARAMS_GAS_IN
182:                 ],
183:                 dParams,
184:                 _fromChainId
185:             );

266:         DepositMultipleParams memory dParams = _bridgeInMultiple(
267:             _account,
268:             _data[
269:                 PARAMS_START_SIGNED:
270:                     PARAMS_END_SIGNED_OFFSET
271:                         + uint16(uint8(bytes1(_data[PARAMS_START_SIGNED]))) * PARAMS_TKN_SET_SIZE_MULTIPLE
272:             ],
273:             _fromChainId
274:         );

283:                 (success, result) = IRouter(_router).anyExecuteSignedDepositMultiple(
284:                     _data[PARAMS_END_SIGNED_OFFSET
285:                         + uint16(uint8(bytes1(_data[PARAMS_START_SIGNED]))) * PARAMS_TKN_SET_SIZE_MULTIPLE],
286:                     _data[
287:                         PARAMS_START + PARAMS_END_SIGNED_OFFSET
288:                             + uint16(uint8(bytes1(_data[PARAMS_START_SIGNED]))) * PARAMS_TKN_SET_SIZE_MULTIPLE:
289:                             _data.length - PARAMS_GAS_IN
290:                     ],
291:                     dParams,
292:                     _account,
293:                     _fromChainId
294:                 );

L160, L177, L266, L283

File: src/uni-v3-staker/UniswapV3Staker.sol

416:             secondsInsideX128 = RewardMath.computeBoostedSecondsInsideX128(
417:                 stakedDuration,
418:                 liquidity,
419:                 uint128(boostAmount),
420:                 uint128(boostTotalSupply),
421:                 secondsPerLiquidityInsideInitialX128,
422:                 secondsPerLiquidityInsideX128
423:             );

515:             stake.liquidityNoOverflow = uint96(liquidity);

L416, L515

File: src/ulysses-omnichain/RootBridgeAgent.sol

359:         bytes memory data = abi.encodePacked(
360:             bytes1(0x02),
361:             _recipient,
362:             uint8(hTokens.length),
363:             settlementNonce,
364:             hTokens,
365:             tokens,
366:             _amounts,
367:             _deposits,
368:             _data,
369:             _manageGasOut(_toChain)
370:         );

L359

File: src/ulysses-omnichain/BranchBridgeAgent.sol

288:         bytes memory packedData = abi.encodePacked(
289:             bytes1(0x06),
290:             msg.sender,
291:             uint8(_dParams.hTokens.length),
292:             depositNonce,
293:             _dParams.hTokens,
294:             _dParams.tokens,
295:             _dParams.amounts,
296:             _deposits,
297:             _dParams.toChain,
298:             _params,
299:             msg.value.toUint128(),
300:             _remoteExecutionGas
301:         );

332:         if (uint8(getDeposit[_depositNonce].hTokens.length) == 1) {

365:         } else if (uint8(getDeposit[_depositNonce].hTokens.length) > 1) {

370:                 packedData = abi.encodePacked(
371:                     bytes1(0x06),
372:                     msg.sender,
373:                     uint8(getDeposit[_depositNonce].hTokens.length),
374:                     nonce,
375:                     getDeposit[nonce].hTokens,
376:                     getDeposit[nonce].tokens,
377:                     getDeposit[nonce].amounts,
378:                     _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens),
379:                     _toChain,
380:                     _params,
381:                     msg.value.toUint128(),
382:                     _remoteExecutionGas
383:                 );

385:                 packedData = abi.encodePacked(
386:                     bytes1(0x03),
387:                     uint8(getDeposit[nonce].hTokens.length),
388:                     _depositNonce,
389:                     getDeposit[nonce].hTokens,
390:                     getDeposit[nonce].tokens,
391:                     getDeposit[nonce].amounts,
392:                     _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens),
393:                     _toChain,
394:                     _params,
395:                     msg.value.toUint128(),
396:                     _remoteExecutionGas
397:                 );

724:         bytes memory packedData = abi.encodePacked(
725:             bytes1(0x03),
726:             uint8(_dParams.hTokens.length),
727:             depositNonce,
728:             _dParams.hTokens,
729:             _dParams.tokens,
730:             _dParams.amounts,
731:             deposits,
732:             _dParams.toChain,
733:             _params,
734:             _gasToBridgeOut,
735:             _remoteExecutionGas
736:         );

L288, L332, L365, L370, L385, L724

File: src/talos/base/TalosBaseStrategy.sol

261:             uint128 liquidityToDecrease = uint128((liquidity * shares) / totalSupply);

L261

File: src/uni-v3-staker/libraries/IncentiveTime.sol

20:         return uint96(((timestamp - INCENTIVES_OFFSET) / INCENTIVES_DURATION) * INCENTIVES_DURATION + INCENTIVES_OFFSET);

26:         return uint96(
27:             (((timestamp - INCENTIVES_OFFSET) / INCENTIVES_DURATION) + 1) * INCENTIVES_DURATION + INCENTIVES_OFFSET
28:         );

32:         end = start + uint96(INCENTIVES_DURATION);

41:         end = start + uint96(INCENTIVES_DURATION);

L20, L26, L32, L41

</details>

 

[L-22] Unused Ownable

Ownable is inherited from but the onlyOwner modifier is never used.

<details><summary>Instances: 2</summary>
File: src/gauges/factories/BribesFactory.sol

18: contract BribesFactory is Ownable, IBribesFactory {

L18

File: src/rewards/rewards/FlywheelGaugeRewards.sol

18: contract FlywheelGaugeRewards is Ownable, IFlywheelGaugeRewards {

L18

</details>

 

[L-23] Missing address(0) checks in constructor/initialize

Failing to check for invalid parameters on deployment may result in an erroneous input and require an expensive redeployment.

<details><summary>Instances: 61</summary>
File: src/hermes/tokens/HERMES.sol

48:     constructor(address _owner) ERC20("Hermes", "HERMES", 18) {

L48

File: src/maia/tokens/Maia.sol

41:     constructor(address _owner) ERC20("Maia", "MAIA", 18) {

L41

File: src/rewards/booster/FlywheelBoosterGaugeWeight.sol

48:     constructor(bHermesGauges _bHermesGauges) {

L48

File: src/rewards/depots/SingleRewardsDepot.sol

22:     constructor(address _asset) {

L22

File: src/hermes/tokens/bHermesBoost.sol

22:     constructor(address _owner) ERC20("bHermes Boost", "bHERMES-B", 18) {

L22

File: src/rewards/rewards/FlywheelBribeRewards.sol

27:     constructor(FlywheelCore _flywheel, uint256 _rewardsCycleLength)
28:         FlywheelAcummulatedRewards(_flywheel, _rewardsCycleLength)
29:     {}

L27

File: src/rewards/rewards/FlywheelInstantRewards.sol

28:     constructor(FlywheelCore _flywheel) BaseFlywheelRewards(_flywheel) {

L28

File: src/rewards/FlywheelCoreInstant.sol

33:     constructor(
34:         address _rewardToken,
35:         IFlywheelRewards _flywheelRewards,
36:         IFlywheelBooster _flywheelBooster,
37:         address _owner
38:     ) Core(_rewardToken, address(_flywheelRewards), _flywheelBooster, _owner) {}

L33

File: src/rewards/FlywheelCoreStrategy.sol

32:     constructor(
33:         address _rewardToken,
34:         IFlywheelRewards _flywheelRewards,
35:         IFlywheelBooster _flywheelBooster,
36:         address _owner
37:     ) Core(_rewardToken, address(_flywheelRewards), _flywheelBooster, _owner) {}

L32

File: src/hermes/tokens/bHermesGauges.sol

26:     constructor(address _owner, uint32 _rewardsCycleLength, uint32 _incrementFreezeWindow)
27:         ERC20Gauges(_rewardsCycleLength, _incrementFreezeWindow)
28:         ERC20("bHermes Gauges", "bHERMES-G", 18)
29:     {

L26

File: src/hermes/tokens/bHermesVotes.sol

20:     constructor(address _owner) ERC20("bHermes Votes", "bHERMES-V", 18) {

L20

File: src/talos/factories/TalosStrategyVanillaFactory.sol

24:     constructor(INonfungiblePositionManager _nonfungiblePositionManager, OptimizerFactory _optimizerFactory)
25:         TalosBaseStrategyFactory(_nonfungiblePositionManager, _optimizerFactory)
26:     {}

L24

File: src/talos/factories/BoostAggregatorFactory.sol

34:     constructor(UniswapV3Staker _uniswapV3Staker) {

L34

File: src/gauges/UniswapV3Gauge.sol

33:     constructor(
34:         FlywheelGaugeRewards _flywheelGaugeRewards,
35:         address _uniswapV3Staker,
36:         address _uniswapV3Pool,
37:         uint24 _minimumWidth,
38:         address _owner
39:     ) BaseV2Gauge(_flywheelGaugeRewards, _uniswapV3Pool, _owner) {

L33

File: src/rewards/depots/MultiRewardsDepot.sol

29:     constructor(address _owner) {

L29

File: src/ulysses-omnichain/VirtualAccount.sol

25:     constructor(address _userAddress, address _localPortAddress) {

L25

File: src/talos/factories/TalosStrategyStakedFactory.sol

40:     constructor(
41:         INonfungiblePositionManager _nonfungiblePositionManager,
42:         OptimizerFactory _optimizerFactory,
43:         BoostAggregatorFactory _boostAggregatorFactory
44:     ) TalosBaseStrategyFactory(_nonfungiblePositionManager, _optimizerFactory) {

L40

File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol

35:     function initialize(address _wrappedNativeTokenAddress, address _coreRouter) external onlyOwner {

L35

File: src/maia/factories/PartnerManagerFactory.sol

37:     constructor(ERC20 _bHermes, address _owner) {

L37

File: src/ulysses-amm/UlyssesRouter.sol

20:     constructor(UlyssesFactory _ulyssesFactory) {

L20

File: src/ulysses-omnichain/factories/RootBridgeAgentFactory.sol

48:     constructor(
49:         uint24 _rootChainId,
50:         WETH9 _wrappedNativeToken,
51:         address _localAnyCallAddress,
52:         address _rootPortAddress,
53:         address _daoAddress
54:     ) {

L48

File: src/talos/TalosOptimizer.sol

34:     constructor(
35:         uint32 _twapDuration,
36:         int24 _maxTwapDeviation,
37:         int24 _tickRangeMultiplier,
38:         uint24 _priceImpactPercentage,
39:         uint256 _maxTotalSupply,
40:         address _owner
41:     ) {

L34

File: src/governance/GovernorBravoDelegator.sol

8:     constructor(
9:         address timelock_,
10:         address govToken_,
11:         address admin_,
12:         address implementation_,
13:         uint256 votingPeriod_,
14:         uint256 votingDelay_,
15:         uint256 proposalThreshold_
16:     ) public {

L8

File: src/maia/vMaia.sol

48:     constructor(
49:         PartnerManagerFactory _factory,
50:         uint256 _bHermesRate,
51:         ERC20 _partnerAsset,
52:         string memory _name,
53:         string memory _symbol,
54:         address _bhermes,
55:         address _partnerVault,
56:         address _owner
57:     ) ERC4626PartnerManager(_factory, _bHermesRate, _partnerAsset, _name, _symbol, _bhermes, _partnerVault, _owner) {

L48

File: src/gauges/factories/BribesFactory.sol

50:     constructor(
51:         BaseV2GaugeManager _gaugeManager,
52:         FlywheelBoosterGaugeWeight _flywheelGaugeWeightBooster,
53:         uint256 _rewardsCycleLength,
54:         address _owner
55:     ) {

L50

File: src/gauges/factories/UniswapV3GaugeFactory.sol

50:     constructor(
51:         BaseV2GaugeManager _gaugeManager,
52:         bHermesBoost _bHermesBoost,
53:         IUniswapV3Factory _factory,
54:         INonfungiblePositionManager _nonfungiblePositionManager,
55:         FlywheelGaugeRewards _flywheelGaugeRewards,
56:         BribesFactory _bribesFactory,
57:         address _owner
58:     ) BaseV2GaugeFactory(_gaugeManager, _bHermesBoost, _bribesFactory, _owner) {

L50

File: src/talos/TalosManager.sol

44:     constructor(
45:         address _strategy,
46:         int24 _ticksFromLowerRebalance,
47:         int24 _ticksFromUpperRebalance,
48:         int24 _ticksFromLowerRerange,
49:         int24 _ticksFromUpperRerange
50:     ) {

L44

File: src/ulysses-omnichain/factories/ArbitrumBranchBridgeAgentFactory.sol

54:     function initialize(address _coreRootBridgeAgent) external override onlyOwner {

L54

File: src/hermes/bHermes.sol

55:     constructor(ERC20 _hermes, address _owner, uint32 _gaugeCycleLength, uint32 _incrementFreezeWindow)
56:         UtilityManager(
57:             address(new bHermesGauges(_owner, _gaugeCycleLength, _incrementFreezeWindow)),
58:             address(new bHermesBoost(_owner)),
59:             address(new bHermesVotes(_owner))
60:         )
61:         ERC4626DepositOnly(_hermes, "Burned Hermes: Gov + Yield + Boost", "bHermes")
62:     {}

L55

File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol

38:     constructor(address _hTokenFactoryAddress, address _localPortAddress)
39:         CoreBranchRouter(_hTokenFactoryAddress, _localPortAddress)
40:     {}

L38

File: src/ulysses-amm/UlyssesToken.sol

21:     constructor(
22:         uint256 _id,
23:         address[] memory _assets,
24:         uint256[] memory _weights,
25:         string memory _name,
26:         string memory _symbol,
27:         address _owner
28:     ) ERC4626MultiToken(_assets, _weights, _name, _symbol) {

L21

File: src/gauges/factories/BaseV2GaugeManager.sol

43:     constructor(bHermes _bHermes, address _owner, address _admin) {

L43

File: src/hermes/minters/BaseV2Minter.sol

53:     constructor(
54:         address _vault, // the B(3,3) system that will be locked into
55:         address _dao,
56:         address _owner
57:     ) {

78:     function initialize(FlywheelGaugeRewards _flywheelGaugeRewards) external {

L53, L78

File: src/talos/boost-aggregator/BoostAggregator.sol

64:     constructor(UniswapV3Staker _uniswapV3Staker, ERC20 _hermes, address _owner) {

L64

File: src/talos/TalosStrategyVanilla.sol

58:     constructor(
59:         IUniswapV3Pool _pool,
60:         ITalosOptimizer _optimizer,
61:         INonfungiblePositionManager _nonfungiblePositionManager,
62:         address _strategyManager,
63:         address _owner
64:     ) TalosStrategySimple(_pool, _optimizer, _nonfungiblePositionManager, _strategyManager, _owner) {}

L58

File: src/ulysses-omnichain/ArbitrumBranchPort.sol

33:     constructor(uint24 _localChainId, address _rootPortAddress, address _owner) BranchPort(_owner) {

L33

File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol

71:     constructor(
72:         WETH9 _wrappedNativeToken,
73:         uint256 _localChainId,
74:         address _rootBridgeAgentAddress,
75:         address _localAnyCallAddress,
76:         address _localAnyCallExecutorAddress,
77:         address _localRouterAddress,
78:         address _localPortAddress
79:     )
80:         BranchBridgeAgent(
81:             _wrappedNativeToken,
82:             _localChainId,
83:             _localChainId,
84:             _rootBridgeAgentAddress,
85:             _localAnyCallAddress,
86:             _localAnyCallExecutorAddress,
87:             _localRouterAddress,
88:             _localPortAddress
89:         )
90:     {}

L71

File: src/talos/TalosStrategyStaked.sol

63:     constructor(
64:         IUniswapV3Pool _pool,
65:         ITalosOptimizer _optimizer,
66:         BoostAggregator _boostAggregator,
67:         address _strategyManager,
68:         FlywheelCoreInstant _flywheel,
69:         address _owner
70:     )
71:         TalosStrategySimple(
72:             _pool,
73:             _optimizer,
74:             _boostAggregator.nonfungiblePositionManager(),
75:             _strategyManager,
76:             _owner
77:         )
78:     {

L63

File: src/rewards/rewards/FlywheelGaugeRewards.sol

53:     constructor(address _rewardToken, address _owner, ERC20Gauges _gaugeToken, IBaseV2Minter _minter) {

L53

File: src/ulysses-omnichain/CoreBranchRouter.sol

26:     constructor(address _hTokenFactoryAddress, address _localPortAddress) BaseBranchRouter() {

L26

File: src/ulysses-omnichain/CoreRootRouter.sol

56:     constructor(uint24 _rootChainId, address _wrappedNativeToken, address _rootPortAddress) {

63:     function initialize(address _bridgeAgentAddress, address _hTokenFactory) external onlyOwner {

L56, L63

File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol

61:     constructor(address owner) {

L61

File: src/uni-v3-staker/UniswapV3Staker.sol

114:     constructor(
115:         IUniswapV3Factory _factory,
116:         INonfungiblePositionManager _nonfungiblePositionManager,
117:         IUniswapV3GaugeFactory _uniswapV3GaugeFactory,
118:         bHermesBoost _hermesGaugeBoost,
119:         uint256 _maxIncentiveStartLeadTime,
120:         address _minter,
121:         address _hermes
122:     ) {

L114

File: src/ulysses-amm/UlyssesPool.sol

80:     constructor(
81:         uint256 _id,
82:         address _asset,
83:         string memory _name,
84:         string memory _symbol,
85:         address _owner,
86:         address _factory
87:     ) UlyssesERC4626(_asset, _name, _symbol) {

L80

File: src/ulysses-omnichain/BranchBridgeAgent.sol

141:     constructor(
142:         WETH9 _wrappedNativeToken,
143:         uint256 _rootChainId,
144:         uint256 _localChainId,
145:         address _rootBridgeAgentAddress,
146:         address _localAnyCallAddress,
147:         address _localAnyCallExecutorAddress,
148:         address _localRouterAddress,
149:         address _localPortAddress
150:     ) {

L141

File: src/rewards/base/BaseFlywheelRewards.sol

31:     constructor(FlywheelCore _flywheel) {

L31

File: src/rewards/rewards/FlywheelAcummulatedRewards.sol

33:     constructor(FlywheelCore _flywheel, uint256 _rewardsCycleLength) BaseFlywheelRewards(_flywheel) {

L33

File: src/talos/strategies/TalosStrategySimple.sol

18:     constructor(
19:         IUniswapV3Pool _pool,
20:         ITalosOptimizer _strategy,
21:         INonfungiblePositionManager _nonfungiblePositionManager,
22:         address _strategyManager,
23:         address _owner
24:     ) TalosBaseStrategy(_pool, _strategy, _nonfungiblePositionManager, _strategyManager, _owner) {}

L18

File: src/talos/factories/TalosBaseStrategyFactory.sol

39:     constructor(INonfungiblePositionManager _nonfungiblePositionManager, OptimizerFactory _optimizerFactory) {

L39

File: src/erc-4626/ERC4626DepositOnly.sol

23:     constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {

L23

File: src/erc-4626/UlyssesERC4626.sol

24:     constructor(address _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, 18) {

L24

File: src/gauges/BaseV2Gauge.sol

61:     constructor(FlywheelGaugeRewards _flywheelGaugeRewards, address _strategy, address _owner) {

L61

File: src/hermes/UtilityManager.sol

44:     constructor(address _gaugeWeight, address _gaugeBoost, address _governance) {

L44

File: src/erc-4626/ERC4626.sol

23:     constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {

L23

File: src/gauges/factories/BaseV2GaugeFactory.sol

51:     constructor(
52:         BaseV2GaugeManager _gaugeManager,
53:         bHermesBoost _bHermesBoost,
54:         BribesFactory _bribesFactory,
55:         address _owner
56:     ) {

L51

File: src/maia/PartnerUtilityManager.sol

36:     constructor(
37:         address _gaugeWeight,
38:         address _gaugeBoost,
39:         address _governance,
40:         address _partnerGovernance,
41:         address _partnerVault
42:     ) UtilityManager(_gaugeWeight, _gaugeBoost, _governance) {

L36

File: src/rewards/base/FlywheelCore.sol

45:     constructor(address _rewardToken, address _flywheelRewards, IFlywheelBooster _flywheelBooster, address _owner) {

L45

File: src/maia/tokens/ERC4626PartnerManager.sol

50:     constructor(
51:         PartnerManagerFactory _factory,
52:         uint256 _bHermesRate,
53:         ERC20 _partnerAsset,
54:         string memory _name,
55:         string memory _symbol,
56:         address _bhermes,
57:         address _partnerVault,
58:         address _owner
59:     )
60:         PartnerUtilityManager(
61:             address(bHermes(_bhermes).gaugeWeight()),
62:             address(bHermes(_bhermes).gaugeBoost()),
63:             address(bHermes(_bhermes).governance()),
64:             address(new ERC20MultiVotes(_owner)),
65:             partnerVault
66:         )
67:         ERC4626(
68:             _partnerAsset,
69:             string.concat(_name, " - Burned Hermes: Aggregated Gov + Yield + Boost"),
70:             string.concat(_symbol, "-bHermes")
71:         )
72:     {

L50

File: src/talos/base/TalosBaseStrategy.sol

79:     constructor(
80:         IUniswapV3Pool _pool,
81:         ITalosOptimizer _optimizer,
82:         INonfungiblePositionManager _nonfungiblePositionManager,
83:         address _strategyManager,
84:         address _owner
85:     ) ERC20("TALOS LP", "TLP", 18) {

L79

</details>

 

 

#0 - c4-judge

2023-07-25T14:17:11Z

trust1995 marked the issue as grade-b

#1 - Madalad

2023-07-26T10:13:15Z

Three of my submitted high/medium findings were downgraded to QA and acknowledged:

  • #576
  • #579
  • #584

Is this enough to upgrade my overall QA score from a B to an A?

Findings Information

Awards

195.3093 USDC - $195.31

Labels

grade-b
satisfactory
sponsor confirmed
analysis-advanced
A-10

External Links

Approach taken in evaluating the codebase

Due to the size of the codebase, my approach was to tackle each subprotocol separately, and then consider their interactions once I was familiar with each of them individually. Since I was unable to spend the entire contest time period auditing, I did not manage to thoroughly understand each subprotocol, however I do feel that I had a strong grasp on certain sections by the end of the audit, notably the contracts in the uni-v3-staker and talos folders.

Architecture recommendations

Overall the codebase is nicely written and well thought out. I do however have some feedback considering the UniswapV3Staker contract: rewards are calculated using epochs, and whenever a NonFungiblePositionManager NFT is staked, it is staked for the current epoch. This means that each time a new epoch begins, the user must restake() their NFT in order to continue earning rewards. While the devs have tried to make this simple by omitting access control from the restake() function entirely, it still provides a poor user experience and may lead to users unknowingly sacrificing rewards.

Codebase quality analysis

For the most part the code is well written and follows best practices in terms of style. Contracts are organized in an appropriate and understandable manner, with interface and library contracts correctly having their own folders.

When it comes to interacting with external contracts, in some areas it was clear that the devs did not fully understand the code they were interacting with:

  • use of transfer instead of safeTransfer for ERC20 tokens 1 2
  • failure to utilize Uniswap slippage control 1 2 3 4 5
  • failure to utilize Uniswap's deadline parameter 1 2 3 4 5

Some other best practices that were not followed include:

  • use of floating pragmas for non-abstract non-interface contracts
  • require/revert statements lacking error message
  • assembly blocks lacking descriptive comments
  • naming convention for constant variables (they should be named using UPPERCASE_WITH_UNDERSCORES)

Centralization risks

The Maia ecosystem uses trusted admins throughout the protocol. Centralization risks are outlined below:

  • admin may mint arbitrary amount of HERMES, Maia ERC20 tokens, diluting the supply and stealing value from holders
  • admin may remove assets from MultiRewardsDepot, UlyssesToken at will, potentially catching users off guard
  • admin may set an arbitrary dao share in BaseV2Minter through frontrunning to mislead users and steal funds
  • admin controls weight and fees of UlyssesPool, which can be changed at any point to the detriment of users

Systemic risks

  • Epochs in gauge contracts inheriting from BaseV2Gauge must each be started manually by calling newEpoch(). If keepers are intended to be used for this, then the protocol is dependent on those keepers remaining functional, as if they don't user rewards could be lost. This risk is mitigated somewhat by allowing anyone to call the newEpoch() function, however relying on an EOA for this purpose is not reliable.
  • Contract UniswapV3Staker allows users to deposit Uniswap V3 pool positions in order to earn incentivized rewards. Should a pool become very illiquid/inactive, adverse consequences could occur.
  • The omnichain functionality of the protocol inherently relies on a messenger to relay messages from one blockchain to another, which must occur off chain. When using this functionality, users must be aware of the fact that they are trusting this entity to be operating correctly. Failure here likely results in lost funds.

Time spent:

25 hours

#0 - c4-judge

2023-07-11T08:37:08Z

trust1995 marked the issue as grade-b

#1 - c4-judge

2023-07-11T08:37:13Z

trust1995 marked the issue as satisfactory

#2 - c4-sponsor

2023-07-12T23:13:50Z

0xBugsy marked the issue as sponsor confirmed

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter