TraderDAOai smart contract security audit report performed by Callisto Security Audit Department
-
Webpage: https://traderdao.ai/
-
Discord: https://discord.gg/traderdao
-
Twitter: https://twitter.com/traderdao_ai
-
Telegram: https://t.me/traderdaoai
-
Medium: https://medium.com/@traderdao-ai
-
Litepaper: https://traderdao.gitbook.io/traderdao/traderdao/litepaper-english
-
Chain to deploy: Arbitrum (ARB)
Audited contracts don't implement the functionality described in the Litepaper; therefore, it uses centralized server-side for all TraderDAO
logic.
Commit: 41f87758492e7af5f3f51b6d68b48069bd70b74e
In total, 1 issue were reported, including:
-
0 high severity issues.
-
0 medium severity issues.
-
0 low severity issues.
In total, 5 notes were reported, including:
-
4 notes.
-
16 owner privileges.
3.1. Owner privileges of Ambassador_Redeem_Contract
- Contract
Ambassador_Redeem_Contract
contract inherits basic access control properties from Openzeppelin's Ownable contract, where the contract's ownership can be transferred or renounced. Renouncing ownership will leave the contract without an owner, thereby disabling any functionality that is only available to the owner. - Function
SetPause()
allows the owner to pause or resume the contracts functionalities available to the users. Users would be unable to redeem USDT tokes for the signature signed by thesignerAddress
if the contract is paused. - Function
SetSigner()
allows the owner to update thesignerAddress
. All previous un-redeemed signatures will be invalid if thesignerAddress
is updated. New signatures must be generated for previous unclaimed signatures. - Function
Save()
allows owner to withdraw ERC-20 tokens from the contract.
3.2. Owner privileges of Liquidity_Wallet
The functions SetDecimal()
and SetRate()
allow the gov
address to modify POT<>USDT conversion parameters. With current values of Rate = 100
and Decimals = 10**18
, the 1 POT = 0.0001 USDT
Users should know that the gov
address can set any conversion rate without restriction.
- Contract
Liquidity_Wallet
contract inherits basic access control properties from Openzeppelin's Ownable contract, where the contract's ownership can be transferred or renounced. Renouncing ownership will leave the contract without an owner, thereby disabling any functionality that is only available to the owner. - Function
SetPause()
allows the owner to pause or resume the contracts functionalities available to the users. Users cannot trade POT tokens for USDT tokens if the contract is paused. - Function
SetGov()
allows the owner to update thegov
address. - Function
Save()
allows owner to withdraw ERC-20 tokens from the contract. - Function
SetFee()
allow thegov
address to set fee for user's claim amount.
3.3. POT_Token.sol
- Contract
POT_Token
contract inherits basic access control properties from Openzeppelin's Ownable contract, where the contract's ownership can be transferred or renounced. Renouncing ownership will leave the contract without an owner, thereby disabling any functionality that is only available to the owner. - Function
ownerMint
allows the owner to mint arbitrary tokens. - Function
SetSigner()
allows the owner to update thesignerAddress
. All previous un-redeemed signatures will be invalid if thesignerAddress
is updated. New signatures must be generated for previous unclaimed signatures.
Based on the docs, the Proof_of_Trade_Arbi_One contract should rewards users in POT tokens, but the contract's implementation rewards users in USDT tokens.
Consider reviewing the implementation or updating documentation based on the business requirements.
- Function
SetSigner()
allows the owner to update thesignerAddress
. All previous un-redeemed signatures will be invalid if thesignerAddress
is updated. New signatures must be generated for previous unclaimed signatures. - Function
SetPause()
allows the owner to pause or resume the contracts functionalities available to the users. Users cannot deposit USDT tokens or Claim rewards if the contract is paused. - Function
Save()
allows owner to withdraw ERC-20 tokens from the contract.
The function deposit()
can be called by any user with any parameters. It allows an attacker to deposit with id_
that already exists (duplicate deposit).
Since all logic is based on the backend, we can't say what severity it has.
In the current implementation there isn't this issue, but if you want to reuse this code, please, pay attention to the following:
-
Since message hash
getMessage(timestamp_, amount_, msg.sender)
has the same format forAmbassador_Redeem_Contract
,POT_Token
,Proof_Of_Trade_Arbi_One
, thesignerAddress
must be different for each contract to avoid replaying signature. -
The message hash generation also does not append any blockchain-specific information (PREFIX) to the message hash, allowing newly generated signatures to be replayed across many chains in case of deploying these contracts on other chains.
- Missing docstrings
The contracts in the code base lack documentation. This hinders reviewers’ understanding of the code’s intention, which is fundamental to correctly assess not only security but also correctness. Additionally, 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).
- Missing test suite
The contract is missing a test suite to validate and verify the behavior of the contract functionalities. Add tests are recommended to ensure that the contract functions and behaves as expected.
- Missing zero address checks
In several places in the code, addresses are passed as parameters to functions. In many of these instances, the functions do not validate that the passed address is not the address 0. While this does not currently pose a security risk, consider adding checks for the passed addresses being nonzero to prevent unexpected behavior where required, or documenting the fact that a zero address is indeed a valid parameter.
- Insufficient event logging
Multiple functions with owner privileges in the contracts do not emit an event. Events are useful to inform external dapps or users that an important state was modified on the contract.
- 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 being withdrawn in a short period of time until the owner or the community of the contract approves further operations.
- Multisig owner account.
- Standard ERC20-related issues. - 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 token 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 schemekeccak256(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 thenonce
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.
The audited smart contract can be deployed. Pay attention to issue 3.4.3, it can have affect on backend part of project.
Users should be aware of the complete centralization of TraderDAO
, where the owner can withdraw any tokens from smart contacts without limitation. Users can claim USDT from TraderDAO
only if the owner adds enough USDT to contracts. The owner can mint POT
tokens without restriction.
Audited contracts don't implement the functionality described in the Litepaper; therefore, it uses centralized server-side for all TraderDAO
logic.
It is recommended to adhere to the security practices described in pt. 4 of this report to ensure the contract's operability and prevent any issues that are not directly related to the code of this smart contract.