LinoSwap Ⅱ

CertiK23 Feb 2023Files named in the report

Audit

CertiK reviewed LinoSwap (LinoSwap Ⅱ) on 23 Feb 2023: 17 findings, 0 critical; 15 marked fixed, 1 acknowledged, 1 open.

No open critical findings

Open the report Open LinoSwap’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings17

Statuses are the security firm’s own, from its report.

Severity	Open	Acknowledged	Fixed
Critical	0	0	0
High	0	1	0
Medium	0	0	1
Low	1	0	14

Medium`refreshBenchmarkNFT()` Changes Constant ProductFixed

The function `refreshBenchmarkNFT()` changes the `_reservedBenchmarkNFT` variable and does not affect the `_reservedETH` variable. Thus, this function will alter the constant product curve by either increasing or decreasing the value of `k`. While the value of NFT's may change due to their rarity changing, this function can create an imbalance in the pool.

Low`feeTotalShare` May Exceed The `LinoswapV1Const.TFixed

When the `feeShare` and `royaltyShare` are set, it is only checked that they are individually less than or equal to `LinoswapV1Const.TOTAL_SHARE`. However when the fees for swaps are calculated, the `feeTotalShare` may be the sum of these values which could exceed the `LinoswapV1Const.TOTAL_SHARE`.

Low`public` Function Can Be Declared As `external`Fixed

The function `validate()` in the contract `XykCurve` is never called internally and can be declared `external` as opposed to `public`.

Low`royaltyShare` Can Be Made `uint128`Fixed

In `LinoswapV1Royalty`, the public variable `royaltyShare` can be made a `uint128` to avoid needing to be cast as a `uint128` in `LinoswapV1Pool`. In addition the intermediate variable used in the functions can also be made `uint128`

Low`swapForNFT()` And `swapForNFTPartly()` InputsFixed

The functions `swapForNFT()` and `swapForNFTPartly()` differ in how the NFT ids that are to be swapped out are input. `swapForNFT()` has a LiquidityNFT struct as input, which includes a signature that must be verified. While `swapForNFTPartly()` simply has an array `uint256[]` as input. As `swapForNFT()` has a signature it only allows users to swap for NFT's that a signer has signed for. While `swapForNFTPartly()` allows a user to swap for any NFT held by the pool, without needing a signature. In previous iterations, the signature was needed as the rarityRankPercentage of each NFT had to be provided. However, now the contract stores the value of each NFT via the `benchMarkNFTArray[]`, which can be updated via the `refreshBenchmarkNFT()` function if rarity rank percentages change. Considering this change, a signature is no longer necessary as any NFT held in the pool should be eligible for a swap.

LowDead CodeFixed

In the contract `LinoswapV1Royalty`, there is commented out code that seems to be outdated and unused.

LowInefficient Memory ParameterFixed

In the contract `ParetoDistribution`, the functions `getRarityValue()` and `getRarityValyeArray()` use the `memory` data location for the input `rarityRankPercentage`. However, these inputs are never modified and their functions are never called internally within the contract. Thus, their data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory.

LowLibrary `TransferHelper ` Is UnusedFixed

The library `TransferHelper` is no longer used and seems to have been replaced with `SafeTransferLib`.

LowLocked EtherFixed

The contract `LinoswapV1Factory` has a `receive()` function with a comment stating it is to receive protocol fees. However, protocol fees are now collected via the pools themselves, so this contract no longer requires a `receive()` function. In addition, there is no withdrawal function for this contract, so any native tokens that are sent to the contract will be locked.

LowMissing Input ValidationFixed

In the contract `LinoswapV1Router`: - The following functions do not check that the input `pool` is a registered pool of the factory: - `addLiquidity()`; - `removeLiquidity()`; - `removeLiquidityForETH()`; - `removeAllLiquidity()`; - `swapForNFT()`; - `swapForNFTPartly()`; - `swapForETH()`. --- In the contract `LinoswapV1Factory: - The function `addPools()` adds an address of a pool to an unused `poolID`. When pools are created, their `poolID`'s are generated as the hash of the pools `nft`, `basicCurve`, `rarityFunction`, `rarityValueCurve`, `vMax`, and `feeShare`. It is not checked that the the hash of these values from the input `_poolAddrs`, matches the input `_poolIDs`.

Show 5 more

LowMissing Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. In the contract `LinoswapV1Factory`: - In the function `setPoolTemplate()`, the input `_poolTemplate` should be checked it is not the zero address. --- In the contract `LinoswapV1Router`: - In the function `batchTransfer()`, the input `recipient` should be checked it is not the zero address.

LowUnchecked Low-level CallFixed

The low-level `call` function returns the status of the call as first variable in the returned tuple. The status of the `call` is not asserted to be `true`, which would treat the low-level call as a success even when it reverted. ```solidity=271 address(tokenContract).call( abi.encodeWithSignature("transferFrom(address,address,uint256)", msg.sender, recipient, tokenIds[index]) ); ```

LowUnprotected InitializerFixed

The contract `LinoswapV1Pool` does not protect its initializer. An attacker can call the initializer and change the storage of the logic contract. The clones use their own storage, so there is no risk to the pools generated by the factory, but this can cause confusion if a user sets values for the logic contract and tries to claim they hold for any of the pools created by the factory.

LowUnsafe Integer CastFixed

In the `_updateReserved()` function both the `amountETH` and `amountBenchmarkNFT` are of type uint256 being cast to a smaller type of `uint128` which may truncate the data. In the `_refreshBenchmarkNFT()` function both the `newBenchmarkNFT` and `currentBenchmarkNFT` are of type uint256 being cast to a smaller type of `uint128` which may truncate the data. We understand that it is very unlikely that these values will every exceed the maximum uint128, but it is possible if the supply of ETH becomes larger or if the value of the NFT's is mistakingly given a large value.

LowUnused EventFixed

The event `RarityRankPercentageUpdate` is declared in `ILinoswapV1Pool` but never emitted.