Read the findings of the 2022 Web3 Security Report

In the contract `Babbu`, the role `owner` has authority over the following functions:
- setNftPoolAddress
- setTakeFee
- mint
- withdraw
- withdrawErc20

In the contract `BabbuNFT`, the role `owner` has authority over the following functions:
- setBlindBoxPrice
- setBnbBlindBoxPrice
- setBlindBoxOpenFee
- setBlindBoxBuyLimit
- setToken
- setBnbToken
- awardItemFromOwner
- awardBoxFromOwner
- withdraw
- withdrawErc20
- setBlindboxEnable

In the contract `Marketplace`, the role `owner` has authority over the following functions:
- setTax
- setToken
- setNft
- withdraw
- withdrawErc20

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

Centralization Related Risks

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. So the ERC20 token used in  contract `AuctionHouse` and `Marketplace` can not be deflationary.

For example, if a user uses 100 deflationary tokens (with a 10% transaction fee) to join an auction, only 90 tokens actually arrived in the contract `AuctionHouse`. However, contract `AuctionHouse`  still needs to send 100 tokens to the user, which causes the contract to lose 10 tokens in such a transaction.

Incompatibility With Deflationary Tokens

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 `insuranceFee` is transferred to the `previousBidder`. If there is only one bidder and the `previousBidder` is zero address, the `insuranceFee` will be locked in the contract and can not be withdarwn.
```solidity=151
if (auction[_auctionId].previousBidder != address(0)) {
    token.transfer(auction[_auctionId].previousBidder, insuranceFee);
}
```

Non-withdrawable Tokens

The auction can only be terminated by the auctioneer. If the auction is never terminated by the auctioneer, the last bidder will never receive NFT or get his tokens back.

The Way of Finishing Auctions

The `endTime` is never used after setting. Joining auction should occur before the end time of auction. 

`endTime` Not Used

The `developerReceive` is better to set as `auctionServiceFee - blackHoleReceive - nftFarmingPoolReceive` to eliminate the risk of losing accuracy.

Potential Loss of Precision

The `DEVELOPER_ADDRESS` and `NFT_FARMING_POOL_ADDRESS` are initial as dead address.

Address Initialization

Before calling function `IPancakeFactory.getPair` to get pair, the function `IPancakeFactory.createPair` needs to be called first to create the `BABBU-wBNB` pair.

Pair Not Created

The function `router.swapExactTokensForTokens` called in function `_transfer` swaps `BABBU` token for wBNB to the `BABBU` contract, which will be failed and the EVM will revert the transaction. Beacuse the function `swap` in `UniswapV2Pair` will check for the `to != _token0 && to != _token1`.

```solidity
function swap(uint amount0Out, uint amount1Out, address to, bytes calldata data) external lock {
    require(amount0Out > 0 || amount1Out > 0, 'UniswapV2: INSUFFICIENT_OUTPUT_AMOUNT');
    (uint112 _reserve0, uint112 _reserve1,) = getReserves(); // gas savings
    require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');

    uint balance0;
    uint balance1;
    { // scope for _token{0,1}, avoids stack too deep errors
    address _token0 = token0;
    address _token1 = token1;
    require(to != _token0 && to != _token1, 'UniswapV2: INVALID_TO');
    ...
```

Reference:
- https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router02.sol
- https://github.com/Uniswap/v2-core/blob/master/contracts/UniswapV2Pair.sol

Fail to Swap BABBU for wBNB

The initial supply tokens are minted to the contract deployer, which may raise the community's concerns about the centralization issue.

Token Minted To Centralized Address

The tax can be set without limit when initializing, and can not be changed any more. It is best to set a cap to prevent excessive tax, such as 15%. Otherwise, the tax fee even may be more than transaction amount since it is charged from sender account directly.
```solidity=90
uint256 allFee = amount.mul(taxFee[0].add(taxFee[1]).add(taxFee[2])).div(100);
super._transfer(sender, address(this), allFee);
```

No limit on Tax

The sender will be removed from holder list when the transfer amount is his total balance. However, his balance may not be zero after this total balance transaction. When distributing hold fee as dividend to all the holders, the sender is not excluded and still holds tokens which needs to be sent to recipient. So the sender will also receive his portion of the dividend. This means he still holds tokens. It is unreasonable to remove him from the holder list at this time.

Removing Holder Mistakenly

The given inputs of the constructor are missing the input validations. The address should not be zero address.

Missing Input Validation

The `pairFee.div(2)` is better to calculated as `pairFee - pairFee.div(2)` to eliminate the risk of losing accuracy.

The `holdFee` is better to calculated as `allFee - nftFee - pairFee` to eliminate the risk of losing accuracy.

The function `transferToHolder` called in function `_transfer` is sending reward to every holders. If the number of holders is large enough, it will cost a lot of gas and execution time.

Gas and Execution Time Optimization

In the constructor, the contract approved the router a large amount of allowance to transfer `BABBU` and `WBNB`. However, there is still a risk of the allowance being used up as time goes by and the allowance continues to decrease.

Insufficient Approval

The variable `senderBalanceBeforeTax` is declared but never used or updated.

Unused Variable

The address `taxPool` is used to receive one of tax fees, but it is not initialized in constructor.

Tax Pool Address Initialization

The given input `to` is missing the check for the non-zero address.

The function that affects the status of sensitive variables should be able to emit events as notifications.
- setBlindBoxPrice
- setBnbBlindBoxPrice
- setBlindBoxOpenFee
- setBlindBoxBuyLimit
- setToken
- setBnbToken

Missing Emit Events

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

Missing Error Messages

To make suer the zero address can not be set as `Holder`, it is better to check for the given input of  function `_add` and `_remove` not to be zero address.

The tax can be set without limit. It is best to set a cap to prevent excessive tax, such as 15%.

When purchasing NFT, users may accidentally pay more than the actual price.
```solidity=104
require(listDetail[_tokenId].price <= _price, "Minimum price has not been reached");
```

Possible Overcharging

Not all NFT holdings of one user are listed. So it is incorrect to return a list at the size of all holding NFTs.

Getting Listed NFT Mistakenly

The list detail is not removed from array `listDetail` when unlisting NFT in function `unlistNft`. This may lead to the wrong return value of function `getListedNft`.

`ListDetail` Not Removed When Unlisting NFT

The contract without function `receive()` can not receive the native tokens. So the function `withdraw` will be useless.

Babbu

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

28

0

11

1

16