Community Alert: NFT Pledge-Borrow exploit

The contract is serving as the underlying entity to interact with third-party protocols. The scope of the audit treats 3rd party entities as black boxes and assume their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

The linked variables could be declared as `constant` since these state variables are never modified.

Variables That Could Be Declared as `constant`

In the contract `BaseJumpRateModelV2`, the role `owner` has authority over the following functions:
- function updateJumpRateModel(uint baseRatePerYear, uint multiplierPerYear, uint jumpMultiplierPerYear, uint kink_)

In the contract `LiquidityMining`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `LiquidityMining`, the role `admin` has authority over the following functions:
- function setPendingAdmin(address payable newPendingAdmin)
- function setController(address _controller)
- function setErc20Token(IERC20 _erc20Token)
- function setBorrowPerBlockReward(uint256 _borrowPerBlockReward)
- function setSupplyPerBlockRewardMap(address xToken, uint256 perBlockReward)
- function addPool(address xToken, address collection, uint256 allocPoint, bool isBorrow)
- function setPool(address xToken, address collection, uint256 allocPoint, bool isBorrow)
- function updateBorrow(address xToken, address collection, uint256 amount, address account, uint256 orderId, bool isDeposit)
- function updateSupply(address xToken, uint256 amount, address account, bool isDeposit)

In the contract `LiquidityMining`, the role `controller` has authority over the following functions:
- function updateBorrow(address xToken, address collection, uint256 amount, address account, uint256 orderId, bool isDeposit)
- function updateSupply(address xToken, uint256 amount, address account, bool isDeposit)

In the contract `P2Controller`, the role `admin` has authority over the following functions:
- function addPool(address xToken, uint256 _borrowCap, uint256 _supplyCap)
- function addCollateral(address _collection, uint256 _collateralFactor, uint256 _liquidateFactor, address[] calldata _pools)
- function setCollateralState(address _collection, uint256 _collateralFactor, uint256 _liquidateFactor)
- function setCollateralSupportPools(address _collection, address[] calldata _pools)
- function setOracle(address _oracle)
- function setXNFT(address _xNFT)
- function setLiquidityMining(ILiquidityMining _liquidityMining)
- function setPendingAdmin(address newPendingAdmin)
- function setPause(address xToken, uint256 index, bool isPause)

In the contract `P2Controller`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `PriceOracle`, the role `admin` has authority over the following functions:
- function setPendingAdmin(address newPendingAdmin)
- function setPrice(address[] memory collections, address denotedToken, uint256[] memory _prices)

In the contract `PriceOracle`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `XAirDrop`, the role `admin` has authority over the following functions:
- function setPendingAdmin(address newPendingAdmin)
- function acceptAdmin()
- function setXNFT(address _xNFT)
- function addERC(address nftContract, bool _isListed, uint256 _ercType, address _airDropTokenContract, address _airDropContract, string memory _method, IAdapter _adapter)
- function setERC(address nftContract, bool _isListed, uint256 _ercType, address _airDropTokenContract, address _airDropContract, string memory _method, IAdapter _adapter)

In the contract `XAirDrop`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `XAirDrop`, the role `xNFT` has authority over the following functions:
- function execution(address nftContract, address airDropContract, address receiver, uint256 tokenId)

In the contract `XNFT`, the role `admin` has authority over the following functions:
- function setCollectionlWhitList(address collection, bool flag, uint256 _auctionDuration, uint256 _redeemProtection, uint256 _increasingMin)
- function batchAddCollectionlWhitList(address[] calldata collections, uint256[] calldata _auctionDuration, uint256[] calldata _redeemProtection, uint256[] calldata _increasingMin)
- function setPendingAdmin(address newPendingAdmin)
- function setController(address _controller)
- function setPledgerFineRate(uint256 _pledgerFineRate)
- function setRewardFirstRate(uint256 _rewardFirstRate)
- function setRewardLastRate(uint256 _rewardLastRate)
- function setCompensatePledgerRate(uint256 _compensatePledgerRate)
- function setAuctionDurationOverAll(uint256 _auctionDurationOverAll)
- function setRedeemProtectionOverAll(uint256 _redeemProtectionOverAll)
- function setIncreasingMinOverAll(uint256 _increasingMinOverAll)
- function withdraw(address xToken, uint256 amount)
- function setTransferEthGasCost(uint256 _transferEthGasCost)
- function setPause(uint256 index, bool isPause)
- function setXAirDrop(IXAirDrop _xAirDrop)
- function claim(address airdop, bytes memory byteCode)
- function setPunks(IPunks _punks, IWrappedPunks _wrappedPunks)

In the contract `XNFT`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `XToken`, the role `admin` has authority over the following functions:
- function setPendingAdmin(address payable newPendingAdmin)
- function setController(address _controller)
- function setReserveFactor(uint256 newReserveFactor)
- function reduceReserves(uint256 reduceAmount)
- function setInterestRateModel(IInterestRateModel newInterestRateModel)
- function setTransferEthGasCost(uint256 _transferEthGasCost)

In the contract `XToken`, the role `pendingAdmin` has authority over the following functions:
- function acceptAdmin()

In the contract `MAYCSerumAdapter`, the role `admin` has authority over the following functions:
- function setXAirDrop(address _xAirDrop)

Any compromise to these accounts may allow a hacker to take advantage of this authority.

Centralization Related Risks

The function `initialize()` is `public` and can be called by anyone as long as the contract is deployed.

`initialize()` Is Unprotected

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.

Variables That Could Be Declared as Immutable

```solidity=72
    /**
     * @notice Calculates the utilization rate of the market: `borrows / (cash + borrows - reserves)`
     * @param cash The amount of cash in the market
     * @param borrows The amount of borrows in the market
     * @param reserves The amount of reserves in the market (currently unused)
     * @return The utilization rate as a mantissa between [0, 1e18]
     */
    function utilizationRate(uint cash, uint borrows, uint reserves) public pure returns (uint) {
        // Utilization rate is 0 when there are no borrows
        if (borrows == 0) {
            return 0;
        }

        return borrows.mul(1e18).div(cash.add(borrows));
    }
```
Referring to line 73 comments, the utilization rate of the market is `borrows / (cash + borrows - reserves)`. But currently, the result is `borrows.mul(1e18).div(cash.add(borrows))`. If `reserves` is not subtracted, the result will be smaller. Is this as expected?  And this implementation is not the same as  in contract `WhitePaperInterestRateModel`.

Function `utilizationRate()` in Contract `BaseJumpRateModelV2`

One or more state variables are never used in the codebase.

Variable `doubleScale` in `Exponential` is never used.

```solidity
    uint256 constant doubleScale = 1e36;
```

Unused State Variable

When the same pool is added more than once in function `addPool()`, the total amount of reward in function `updatePool()` will be incorrectly calculated. The current implementation is relying on the operation correctness to avoid repeatedly adding the same pool, as the function will only be called by the admin.

`addPool()` Function Not Restricted

In function `setPool`, missing a check to ensure the updated pool already exists. In addition, if `xToken` is changed, the pool is changed to another pool. So the assignment to `poolInfo.xToken` is redundant.

`setPool()` Function Not Restricted

When updating `borrowPerBlockReward/supplyPerBlockRewardMap[xToken]`, the reward for each block will change, the interval for which the reward is not calculated before the update should still be calculated based on the old reward for each block.

In function `setPool()`, all the pools need to be updated before updating `allocPoint`. Otherwise, the `borrowTotalAllocPoint` will change, and the ratio of `pool.allocPoint` and `borrowTotalAllocPoint` will also change. So the `reward` allocated to every pool will be incorrect.

Missing Update Pools Before `setPool()`

Function `mint` has no access restrictions, anyone can mint any tokens to any account.

`mint()` Function Not Restricted

The admin can set or update `collateralFactor/liquidateFactor`, but there is no upper limit on what the factor can be. In the extreme case, the factor can be as high as 100%.

No Upper Limit for `collateralFactor/liquidateFactor`

It's necessary to check if `erc20Map` or `erc721Map` do not exist in function `addERC`.

`addERC()` Function Not Restricted

It's necessary to check if `erc20Map` or `erc721Map` exist in function `setERC`.

`setERC()` Function Not Restricted

`isClaim20AirDropMap` and `isClaim721AirDropMap` are set to true after transferring tokens to `receiver`. But they are not checked at the beginning of the functions `erc20` and `erc721`. So `XNFT.airDrop` can be called multiple times. Users can get airdrops more than one time.

Missing Validation In `airDrop`

The air drop contract for `nftContract` is not unique in the current codebase, one `nftContract` can have multiple air drop contracts. Does this fit your design？

Uniqueness Of Air Drop Contract

The return value is a dynamic array that needs to be initialized to determine the length, and assigning a value to an element directly without initialization will report an error.

Missing Initialization for Array Length

The admin can set `pledgerFineRate/rewardFirstRate/rewardLastRate/compensatePledgerRate`, but there is no upper limit on what the rate can be. In the extreme case, the rate can be as high as 100%.

No Upper Limit for Fee Rate

There is no validation to check whether `collections.length`, `_auctionDuration.length`, `_redeemProtection.length` and `_increasingMin.length` are the same.

Lack Validation for Array Length

xCarnival Ⅴ

MAYCSerumAdapter.sol

BAYCLandAdapter.sol

XTokenStorage.sol

WhitePaperInterestRateModel.sol

XAirDrop.sol

P2Controller.sol

Exponential.sol

Address.sol

XToken.sol

NFT.sol

XNFT.sol

LiquidityMining.sol

BaseJumpRateModelV2.sol

APEAdapter.sol

P2ControllerStorage.sol

JumpRateModelV2.sol

SafeMath.sol

SafeERC20.sol

PriceOracle.sol

Missing input validation

Missing emit events

Centralization risk

External dependency

Logical issue of `IPubSubMgr.publish()`

Gas consumption in loop

Risk of supported NFT digital assets

Incorrect parameter of event `ClaimWithdrawReward`

Restriction on `nftAddresses` may cause calling fail in `Broker`

Redundant variable

Uncertain addresses

Logical issue of `pool.lendTokenBal`

Source of reward token

xCarnival IV

BrokerV2.sol

XCVFaimV2.sol

Unlocked compiler version

Contract accepts arbitrary `ether`

Third party dependencies

Timestamp dependence

Uncertain use of `_tokens` set

Price oracle feed

`_deposit()` function not restricted

Logical issue of `_whitelistMap`

xCarnival III

xcarnival_3

ERC20.sol

IPriceOracles.sol

EthAddressLib.sol

OracleProxy.sol

IERC20.sol

Median.sol

xbrokerfarming-main

Ownable.sol

Context.sol

interface

IOracleProxy.sol

EnumerableSet.sol

ChainlinkWrapper.sol

XCVFarm.sol

library

xcarnival-PriceOracle-main

Proper usage of “public” and “external” type

Lack of input validation

Unknown implementation in the function `publish()`

Redundant codes

Lack of upper boundary

Lack of lower boundary

Logical issue of function `removeToken()`

Unlocked compiler version declaration

Redundant Codes

Megabox

IAsset.sol

Coin.sol

IMintable.sol

Position.sol

IPosition.sol

.gitignore

truffle-config.js

IBurnable.sol

Main.sol

IBalance.sol

2_deploy.js

IERC20Decimals.sol

4_init_config.js

Migrations.sol

package-lock.json

IOracle.sol

package.json

LICENSE

Broker.sol

interfaces

Paused.sol

IBroker.sol

WrappedOracle.sol

Published.sol

README.md

Balance.sol

Config.sol

Blacklist.sol

Asset.sol

Owned.sol

migrations

1_init_migration.js

Admined.sol

deploy-config.js

3_init.js

IConfig.sol

5_transfer_ownership.js

topic.txt

Logical issue of function `setBeneficiary()`

Unreasonable upper boundary

Privileged ownership

Logical Issue of `_profit` distribution

XBroker

The Synthetic DeFi Assets Aggregator for Everyone.

InterestRateModel

XCVToken

XCarnival token

XCarnival

AZ WORLD NETWORK

Galaxy Finance 

MANEKI-NEKO

The Plant DAO

Yasha DAO 

Metaverse VR 

static-analysis

onchain-monitoring

social-sentiment

governance-autonomy

market-volatility

safety-assessment

Exchange	Pair	Price	Spread	24H Volume	24H Vol USD	Volume %

No Data

XCarnival

Highlights & Alerts

Long-running Project

Positive Community Sentiment (top 25%)

Community Alert: NFT Pledge-Borrow exploit

Code Audit History

21

0

8

0

13

Skynet

Sentiment Analysis

Market Volatility

Governance & Autonomy