PandAI Earn smart contract security re-audit report performed by Saksham.
https://github.com/pandai-corp/pandai-earn-sc/tree/main/contracts
Since the updater should have the privilege to call state update functions , the function here https://github.com/pandai-corp/pandai-earn-sc/blob/main/contracts/PandAIEarnV1.sol#L170 should be also callable by UPDATER_ROLE instead of just DEFAULT_ADMIN , this hinders centralisation too.
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.
Consider adding checks to verify that the referralAddress Tier has already been initialized.
Eventhough the comments mentions the risk that rewards may be lost if claim is not called prior to calling depositWithReferral and requestWithdraw. Therefor pnding rewards should be stored (for exapmlple, in userMap[msg.sender].referralPendingReward or create separate variable) within these function at the beginning so that users never experience this loss in rewards. A user might not read carefully/neglect the guidlines/comments and hence lose rewards.
Users will loose interest in the project the moment they realise they have lost their rewards.
The function getPandaiWorthOf is used to calculate the Pandai worth per USDT, and is used in functions to calculate fee.
Since this function checks tokens balances of lpAddress it allows attaker to use flash loan price manipulation to pay very small fee.
For example, using intermidiate contract an attaker can:
1.) Use flash loan to drain tokens from one side (i.e. Pandai token) of LP pool 2.) Call PandAiEarn contract to claim or withdraw tokens. Now function getPandaiWorthOf returns very high price of Pandai tokens so attaker pay very small amount of fee. 3.) Returns flash loan.
This issue has medium severity because an attaker should pay fee for flash loan (depends on DEX it's 0.2%-0.3% of loan amount). So if pool has a lot of liquidity manipulation with price may cost more than pay normal Pandai fee.
Instead of relying on one pool for getting the swap price , it is always advisable to rely on multiple or something like chainlink. History have shown how catostrophic these attack can be.
Affected Code:
https://github.com/pandai-corp/pandai-earn-sc/blob/main/contracts/PandAIEarnV1.sol#L379
https://github.com/pandai-corp/pandai-earn-sc/blob/main/contracts/PandAIEarnV1.sol#L264
https://github.com/pandai-corp/pandai-earn-sc/blob/main/contracts/PandAIEarnV1.sol#L429
https://github.com/pandai-corp/pandai-earn-sc/blob/main/contracts/PandAIEarnV1.sol#L444
The team saved gas by packing together the variables in the Tier struct , but that was meant to be an example as every struct can have that optimization and can save gas in the multiples of 20,000.
All the pending reward variables can be made into uint128 as uint256 depicts a value that will bepractically not achieved , moreover all the timestamp variables can be packed in uint64 as timesatamps can be depicted in uint64 for many many upcoming years.
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.) Owner completely responisble to maintain an adequate balance on the contract for users' withdrawals and rewards payouts. Otherwise users' will not be able to withdraw or claim.
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.
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).
3.2 Function depositWithReferral() does not check if the referralAddress Tier is initiated
It is not an issue. Doc was updated: https://docs.pandai.io/products/pandai-earn#referral-program