Table of Contents

• Summary

  • Issue Summary

• High Issues

  • H-1 Unauthorized Reward Claiming in Distribution::claim Function

• Medium Issues

  • M-1: ILayerZeroEndpoint is re-used

  • M-2: Using ERC721::_mint() can be dangerous

  • M-3: Contracts that lock Ether in NonfungiblePositionManagerMock contract

• Low Issues

  • L-1: Deprecated OpenZeppelin functions should not be used

  • L-2: Unsafe ERC20 Operations should not be used

  • L-3: Solidity pragma should be specific, not wide

  • L-4: PUSH0 is not supported by all chains

• Tools Used

Summary

Issue Summary

Category	No. of Issues
High	1
Medium	3
Low	4

High Issues

[H-1] Unauthorized Reward Claiming in Distribution::claim Function

Summary

The identified vulnerability in the smart contract allows an attacker to claim rewards on behalf of another user without proper authorization. The issue arises from the absence of a check to ensure that the caller is the intended user, providing a potential avenue for unauthorized reward claims.

Vulnerability Details

In the claim function of the smart contract, there is a lack of validation to confirm that the caller (msg.sender) matches the specified user (user_). The absence of this check allows an attacker to exploit the function by providing any user's address as the user_ parameter, leading to unauthorized reward claims. the attacker can read any user's state to determine the right time to claim rewards from any user.
it will also manipulate the poolData struct and at the end, the claim function emits an event UserClaimed but the user didn't do that.
There's a severe disruption of protocol functionality or availability.

Configuration

• Check: unauthorized-reward-claim

• Severity: High

• Confidence: High

Proof of Concept: (Proof of Code)

The below test case shows how the attacker can claim the user's reward:

1. Attach a test in Distribution.test.ts in "#claim" section

Test

it('should not allow an attacker to claim on behalf of another user', async () => {

​

await setNextTime(oneHour * 2);

​

// User stakes

await distribution.connect(OWNER).stake(poolId, wei(100));

​

// Fast forward time by one day

await setNextTime(oneDay + oneDay * 2);

​

// Attacker's attempt to claim on behalf of the user

const attackerRewardsClaim = distribution.connect(SECOND).claim(poolId, OWNER.address, { value: wei(1) });

​

// Ensure the transaction reverts with the expected error message

await expect(attackerRewardsClaim).to.be.revertedWith('DS: Caller is not the intended user');

});

2. Run the Specific test

3. You'll get an output that looks like this:

OutPut

Distribution

#claim

1) should not allow an attacker to claim on behalf of another user

​

​

0 passing (38s)

1 failing

​

1) Distribution

#claim

should not allow an attacker to claim on behalf of another user:

AssertionError: Expected transaction to be reverted with reason 'DS: Caller is not the intended user', but it didn't revert

at processTicksAndRejections (node:internal/process/task_queues:95:5)

at async Context.<anonymous> (test/Distribution.test.ts:778:7)

Impact

The vulnerability poses a significant risk as it enables the attacker to claim rewards on behalf of any user without proper authorization. This could lead to financial losses for the affected users and potential disruptions to the protocol's intended operation.

Recommendations

To mitigate the identified vulnerability, it is recommended to include a validation check at the beginning of the claim function to ensure that the caller (msg. sender) matches the specified user (user_).

Mitigation Code

function claim(uint256 poolId_, address user_) external payable poolExists(poolId_) {

// ... existing code

​

require(msg.sender == user_, "DS: Caller is not the intended user");

​

// ... existing code

}

Medium Issues

M-1: ILayerZeroEndpoint is re-used

Summary

Name reused

Vulnerability Details

The vulnerability stems from the reuse of names for the contract ILayerZeroEndpoint. This can lead to compilation artifacts with the duplicate name, making it challenging to analyze the second contract and ILayerZeroEndpoint is used in Line: 130
If a codebase has two contracts the similar names, the compilation artifacts
will not contain one of the contracts with the duplicate name.

Configuration

• Check: name-reused

• Severity: Medium

• Confidence: High

Exploit Scenario:

Bob's hardhat codebase has two contracts named ERC20.
When npx hardhat compile runs, only one of the two contracts will generate artifacts in artifacts/contracts.
As a result, the second contract cannot be analyzed.

Recommendation

To mitigate this issue, it is crucial to rename the contracts with unique and distinct names. This ensures that both contracts generate separate compilation artifacts and can be properly analyzed.

M-2: Using ERC721::_mint() can be dangerous

Vulnerability Details

Using ERC721::_mint() can mint ERC721 tokens to addresses that don't support ERC721 tokens.

Configuration

• Check: dangerous-erc721-mint

• Severity: Medium

• Confidence: High

• Found in contracts/MOR.sol Line: 25

  _mint(account_, amount_);

• Found in contracts/mock/tokens/WStETHMock.sol Line: 16

  _mint(account_, amount_);

• Found in contracts/mock/tokens/WStETHMock.sol Line: 24

  _mint(msg.sender, stETHAmount_);

Recommendation

Use _safeMint() instead of _mint() for ERC721.

M-3: Contracts that lock Ether in NonfungiblePositionManagerMock contract

Summary

Contracts that lock Ether in Line: 7.
these are all true for Line: 7 too.

Vulnerability Details

Contracts, such as the NonfungiblePositionManagerMock, contain a payable function but lack a corresponding withdrawal mechanism. Without a way for users to retrieve their locked Ether, funds sent to these contracts become irretrievable.
Contract with a payable function, but without a withdrawal capacity.

Configuration

• Check: locked-ether

• Severity: Medium

• Confidence: High

Exploit Scenario:

Every Ether sent to Locked will be lost.

Recommendation

Remove the payable attribute or add a withdraw function.

Low Issues

L-1: Deprecated OpenZeppelin functions should not be used

Openzeppelin has deprecated several functions and replaced them with newer versions. Please consult https://docs.openzeppelin.com/

Configuration

• Check: deprecated-openzeppelin-functions

• Severity: Low

• Confidence: High

• Found in contracts/L2TokenReceiver.sol Line: 43

  TransferHelper.safeApprove(params.tokenIn, router, 0);

• Found in contracts/L2TokenReceiver.sol Line: 44

  TransferHelper.safeApprove(params.tokenIn, nonfungiblePositionManager, 0);

• Found in contracts/L2TokenReceiver.sol Line: 48

  TransferHelper.safeApprove(params.tokenOut, nonfungiblePositionManager, 0);

• Found in contracts/L2TokenReceiver.sol Line: 123

  TransferHelper.safeApprove(newParams_.tokenIn, router, type(uint256).max);

• Found in contracts/L2TokenReceiver.sol Line: 124

  TransferHelper.safeApprove(newParams_.tokenIn, nonfungiblePositionManager, type(uint256).max);

• Found in contracts/L2TokenReceiver.sol Line: 126

  TransferHelper.safeApprove(newParams_.tokenOut, nonfungiblePositionManager, type(uint256).max);

L-2: Unsafe ERC20 Operations should not be used

ERC20 functions may not behave as expected. For example: return values are not always meaningful. It is recommended to use OpenZeppelin's SafeERC20 library.

Configuration

• Check: unsafe-erc20-operations

• Severity: Low

• Confidence: High

• Found in contracts/L1Sender.sol Line: 69

  IERC20(unwrappedDepositToken).approve(oldToken_, 0);

this is also found in these contracts:

• contracts/L1Sender.sol Line: 76

• contracts/L1Sender.sol Line: 91

• contracts/L1Sender.sol Line: 95

• contracts/mock/GatewayRouterMock.sol Line: 15

• contracts/mock/SwapRouterMock.sol Line: 9

• contracts/mock/SwapRouterMock.sol Line: 10

• contracts/mock/tokens/WStETHMock.sol Line: 25

L-3: Solidity pragma should be specific, not wide

Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of pragma solidity ^0.8.0;, use pragma solidity 0.8.0;

Configuration

• Check: specific-solidity-pragma

• Severity: Low

• Confidence: High

• Found in contracts/Distribution.sol Line: 2

  pragma solidity ^0.8.20;

this is also found in these contracts:

• contracts/L1Sender.sol Line: 2

• contracts/L2MessageReceiver.sol Line: 2

• contracts/L2TokenReceiver.sol Line: 2

• contracts/MOR.sol Line: 2

• contracts/interfaces/IDistribution.sol Line: 2

• contracts/interfaces/IL1Sender.sol Line: 2

• contracts/interfaces/IL2MessageReceiver.sol Line: 2

• contracts/interfaces/IL2TokenReceiver.sol Line: 2

• contracts/interfaces/IMOR.sol Line: 2

• contracts/interfaces/tokens/IStETH.sol Line: 2

• contracts/interfaces/tokens/IWStETH.sol Line: 2

• contracts/interfaces/uniswap-v3/INonfungiblePositionManager.sol Line: 2

• contracts/libs/LinearDistributionIntervalDecrease.sol Line: 2

• contracts/mock/DistributionV2.sol Line: 2

• contracts/mock/GatewayRouterMock.sol Line: 2

• contracts/mock/L1SenderV2.sol Line: 2

• contracts/mock/L2MessageReceiverV2.sol Line: 2

• contracts/mock/L2TokenReceiverV2.sol Line: 2

• contracts/mock/NonfungiblePositionManagerMock.sol Line: 2

• contracts/mock/SwapRouterMock.sol Line: 2

• contracts/mock/tokens/StETHMock.sol Line: 2

• contracts/mock/tokens/WStETHMock.sol Line: 2

L-4: PUSH0 is not supported by all chains

Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise, deployment of your contracts will fail.

Configuration

• Check: push0-not-supported

• Severity: Low

• Confidence: Medium

• Found in contracts/Distribution.sol Line: 2

  pragma solidity ^0.8.20;

this is also found in these contracts:

• contracts/L1Sender.sol Line: 2

• contracts/L2MessageReceiver.sol Line: 2

• contracts/L2TokenReceiver.sol Line: 2

• contracts/MOR.sol Line: 2

• contracts/interfaces/IDistribution.sol Line: 2

• contracts/interfaces/IL1Sender.sol Line: 2

• contracts/interfaces/IL2MessageReceiver.sol Line: 2

• contracts/interfaces/IL2TokenReceiver.sol Line: 2

• contracts/interfaces/IMOR.sol Line: 2

• contracts/interfaces/tokens/IStETH.sol Line: 2

• contracts/interfaces/tokens/IWStETH.sol Line: 2

• contracts/interfaces/uniswap-v3/INonfungiblePositionManager.sol Line: 2

• contracts/libs/LinearDistributionIntervalDecrease.sol Line: 2

• contracts/mock/DistributionV2.sol Line: 2

• contracts/mock/GatewayRouterMock.sol Line: 2

• contracts/mock/L1SenderV2.sol Line: 2

• contracts/mock/L2MessageReceiverV2.sol Line: 2

• contracts/mock/L2TokenReceiverV2.sol Line: 2

• contracts/mock/NonfungiblePositionManagerMock.sol Line: 2

• contracts/mock/SwapRouterMock.sol Line: 2

• contracts/mock/tokens/StETHMock.sol Line: 2

• contracts/mock/tokens/WStETHMock.sol Line: 2

Tools Used

Most of the time Manual code inspection and analysis were performed to identify the vulnerability but also automated tools were used and performed in this project like Aderyn and Slither and of course AI.