Skip to content

Instantly share code, notes, and snippets.

@chhajershrenik
Created April 18, 2023 16:58
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save chhajershrenik/bea2b3c9b10dc1ce0a9aaa6013b5e4e9 to your computer and use it in GitHub Desktop.
Save chhajershrenik/bea2b3c9b10dc1ce0a9aaa6013b5e4e9 to your computer and use it in GitHub Desktop.
Binance Smart Chain (BSC) PandAI Earn Contracts Security Audit Report

Binance Smart Chain (BSC) PandAI Earn Contracts Security Audit Report.

1. Summary

PandAI Earn smart contracts security audit report performed by chhajershrenik.

2. In scope

Commit: 5c0fdfb6606ed9d4d86fc27861173d4fa9662f1d

3. Findings

In total, 3 issues were reported, including:

  • 0 critical severity issues.

  • 0 high severity issues.

  • 2 medium severity issue.

  • 1 low severity issue.

In total, 9 notes were reported, including:

  • 4 notes.

  • 5 owner privileges.

3.1. Function depositWithReferral() when the amount of USDT tokens is less than $100 for any additional deposits

Severity: medium

Description

If a user tries to deposit any amount less than $100 the function depositWithReferral() reverts even if the total amount of USDT token is greater than the respective tier limit. For example, if a user deposits $500 equivalent USDT tokens in the first deposit and less than $100 worth of USDT tokens in the second deposit. Even though the total amount of USDT token meets the eligibility criteria for the respective Tier the function reverts due to the small deposit check.

Code Snippet

Recommendation

Consider adding checks to prevent the function from reverting if a user has an existing deposit and allowing a deposit of less than $100 if the total amount of the deposit meets the respective Tier criteria.

3.2. Functions withdraw() would result in an unexpected revert

Severity: medium

Description

The function requestWithdraw() allows users to request withdrawal of the deposited USDT tokens after WITHDRAW_PROCESSING_TIME (14 days) using the function withdraw(), the function requestWithdraw() does not check the withdrawal amount with respect to the contract's USDT token balance also the contract does not maintain a state to lock pending user rewards and withdrawals. If the contract balance is less than the withdrawal amount it would result in a revert of the function withdraw().

Code Snippet

3.3. Function depositWithReferral() does not check if the referralAddress Tier is initiated

Severity: low

Description

Based on the docs, the referral address (referralAddress) should be in Tier 1 or higher. The function depositWithReferral() does not check if the referralAddress has made an existing deposit and if a Tier is being assigned to the address. If the Tier is not yet initiated for the referralAddress is not allowed to collect the referral bonus locking out the bonus in the contract.

Code Snippet

Recommendation

Consider adding checks to verify that the referralAddress Tier has already been initialized.

3.4. Treasury can be any admin address

Severity: note

Description

Functions withdrawTreasury() and depositTreasury() are used to manage Treasury funds, but the functions use any admin address in the access list to withdraw or deposit USDT tokens from the contract. Allowing any admin in the access list to manage the Treasury funds.

Recommendation

Consider using a separate wallet to manage Treasury funds to limit the risks and also to make it easier to keep track of Treasury funds.

3.5. Owner Privileges

Severity: owner privileges

Description

  1. Contract PandAIEarn inherits the traits of OpenZeppelin AccessControl contract allowing admin to manage admin's, the role could be renounced leading to locking out of access to critical functions of the contract.
  2. Functions pause() and unpause allows the admin to disable or enable the ability for the user's deposit USDT tokens respectively.
  3. Function setLpAddress() allows the admin to change the liquidity pool address to any wallet, the value of PandAi tokens would be affected which is computed using the function getPandaiWorthOf() based on the amount of USDT tokens and PandAi tokens available in newLpAddress wallet.
  4. Function withdrawTreasury() allows the admin to withdraw USDT tokens from the contract to the admin address.
  5. Function setUserApprovalLevel() allows admin to change the claim status of any wallet address to NotApproved, Approved, and Forbidden limiting the user's ability to withdraw or restrict a user from withdrawing USDT tokens deposited in the contract based on the approval status.

Recommendation

Since the owner has unlimited rights to do everything, the ownership must be transferred to a multi-sig contract.

3.6. Follow good coding practice

Severity: note

Description

  1. Unlocked Pragma

Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the floating pragma, i.e. by not using ^ in pragma solidity ^0.8.9, ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs.

  1. Typo in the function name

The function name getDepositUnlokTimestamp() is misspelled consider correcting the typo to getDepositUnlockTimestamp() in the contract.

  1. Unorganized and non-standardized docstrings

The contracts in the code base contain unorganized and non-standardized docstring does not explain the contract's state and functionalities in detail. This hinders reviewers’ understanding of the code’s intention, which is fundamental to correctly assess not only security but also correctness. Additionally, detailed docstrings improve readability and ease maintenance. They should explicitly explain the purpose or intention of the functions, the scenarios under which they can fail, the roles allowed to call them, the values returned, and the events emitted.

Consider thoroughly documenting all functions (and their parameters) that are part of the contracts’ public API. Functions implementing sensitive functionality, even if not public, should be documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

4. Security practices

  • Open-source contact.
  • The contract should pass a bug bounty after the completion of the security audit.
  • Public testing.
  • Automated anomaly detection systems. - NOT IMPLEMENTED. A simple anomaly detection algorithm is recommended to be implemented to detect behavior that is atypical compared to normal for this contract. For instance, the contract must halt deposits in case a large amount is withdrawn in a short period until the owner or the community of the contract approves further operations.
  • Multisig owner account.
  • Standard ERC20-related issues. - NOT IMPLEMENTED. It is known that every contract can potentially receive an unintended ERC20-token deposit without the ability to reject it even if the contract is not intended to receive or hold tokens. As a result, it is recommended to implement a function that will allow extracting any arbitrary number of tokens from the contract.
  • Crosschain address collisions. ETH, ETC, CLO, etc. It is possible that a transaction can be sent to the address of your contract at another chain (as a result of a user mistake or some software fault). It is recommended that you deploy a "mock contract" that would allow you to withdraw any tokens from that address or prevent any funds deposits. Note that you can reject transactions of native tokens deposited, but you can not reject the deposits of ERC20 tokens. You can use this source code as a mock contract: extractor contract source code. The address of a new contract deployed using CREATE (0xf0) opcode is assigned following this scheme keccak256(rlp([sender, nonce])). Therefore you need to use the same address that was originally used at the main chain to deploy the mock contract at a transaction with the nonce that matches that on the original chain. Example: If you have deployed your main contract with address 0x010101 at your 2021th transaction then you need to increase your nonce of 0x010101 address to 2020 at the chain where your mock contract will be deployed. Then you can deploy your mock contract with your 2021th transaction, and it will receive the same address as your mainnet contract.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment