Closed 0xmaverickk closed 1 year ago
yuriy77k
commented
1 year ago The audit fee is 2420 USDT. You may send USDT (ERC20 or BEP20) to: 0x6317c6944bd1cD3932d062cce39d7Fd602119529 (valid for Ethereum and Binance Smart Chain)
The estimated auditing time - is 14 days after payment.
yuriy77k
commented
1 year ago yuriy77k
commented
1 year ago 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: f9361197082c833146120cb1a29c59c40fc11e8a
In total, 1 issue were reported, including:
0 high severity issues.
0 medium severity issues.
1 low severity issue.
In total, 5 notes were reported, including:
5 notes.
15 owner privileges.
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.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 the signerAddress if the contract is paused.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.Save() allows owner to withdraw ERC-20 tokens from the contract.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.
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.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.SetGov() allows the owner to update the gov address.Save() allows owner to withdraw ERC-20 tokens from the contract.The contract uses the pause variable to determine whether the users can use contract functionalities, but the contract is missing a function to change the contract state.
Consider implementing a function to change the state of the contract by modifying the pause variable.
All ERC-20 token functionalities except function mint() is available to the user when the contract state is set to pause.
Consider implementing checks to restrict users from accessing functions if the contract state is paused based on the requirements.
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.ownerMint allows the owner to mint arbitrary tokens.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress 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.
SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.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.Save() allows owner to withdraw ERC-20 tokens from the contract.ecrecover with known vulnerabilitiesContracts Ambassador_Redeem_Contract, POT_Token and Proof_of_Trade_Arbi_One uses soliditiy's ecrecover() function to recover signer address from the signature. The function ecrecover() is known to have signature malleability issues. However, this issue has not affected audited contracts.
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.
Some tokens (like USDT on the Ethereum chain) don’t correctly implement the ERC20 standard, and their transfer/transferFrom functions return void instead of a success boolean. Calling these functions with the correct ERC20 function signatures will always revert.
Use safeTransfer() and safeTransferFrom() from TransferHelper library for third-party tokens.
function safeTransfer(address token, address to, uint value) internal {
// bytes4(keccak256(bytes('transfer(address,uint256)')));
(bool success, bytes memory data) = token.call(abi.encodeWithSelector(0xa9059cbb, to, value));
require(success && (data.length == 0 || abi.decode(data, (bool))), 'TransferHelper: TRANSFER_FAILED');
}
function safeTransferFrom(address token, address from, address to, uint value) internal {
// bytes4(keccak256(bytes('transferFrom(address,address,uint256)')));
(bool success, bytes memory data) = token.call(abi.encodeWithSelector(0x23b872dd, from, to, value));
require(success && (data.length == 0 || abi.decode(data, (bool))), 'TransferHelper: TRANSFER_FROM_FAILED');
}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).
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.
The contracts contain comments about files imported but were never imported in the contract.
The declared variable, mapping, and functions in the contracts are never utilized and can be safely removed to optimize gas costs during deployment.
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.
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.
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.The audited smart contract can be deployed. Only low severity issue was found during the audit.
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.
yuriy77k
commented
1 year ago 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.
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.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 the signerAddress if the contract is paused.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.Save() allows owner to withdraw ERC-20 tokens from the contract.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.
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.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.SetGov() allows the owner to update the gov address.Save() allows owner to withdraw ERC-20 tokens from the contract.SetFee() allow the gov address to set fee for user's claim amount.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.ownerMint allows the owner to mint arbitrary tokens.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress 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.
SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.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.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 for Ambassador_Redeem_Contract, POT_Token, Proof_Of_Trade_Arbi_One, the signerAddress 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.
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).
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.
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.
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.
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.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.
yuriy77k
commented
1 year ago 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: 9d4f1b1993ed76de25d7f3555a8b4eebdb8ad768
In total, 0 issue were reported, including:
0 high severity issues.
0 medium severity issues.
0 low severity issues.
In total, 19 notes were reported, including:
3 notes.
16 owner privileges.
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.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 the signerAddress if the contract is paused.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.Save() allows owner to withdraw ERC-20 tokens from the contract.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.
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.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.SetGov() allows the owner to update the gov address.Save() allows owner to withdraw ERC-20 tokens from the contract.SetFee() allow the gov address to set fee for user's claim amount.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.ownerMint allows the owner to mint arbitrary tokens.SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress 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.
SetSigner() allows the owner to update the signerAddress. All previous un-redeemed signatures will be invalid if the signerAddress is updated. New signatures must be generated for previous unclaimed signatures.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.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.
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).
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.
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.
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.
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.The audited smart contract can be deployed. No security issues were found in the audited contracts.
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.
Audit request
... Briefly describe your smart-contract and its main purposes here ... These are contracts for TraderDAO utility token minting and distribution, as well as reward redeem purposes.
Source code
... Give a link to the source code of contracts ... https://github.com/TraderDAOai/contracts
Payment plan
... Write [x] at the checkbox of the payment plan that suits your needs ...
Disclosure policy
... Do you want us to publish the report as it is or to notify you privately in case of finding critical mistakes? ... Notify us first, after we fixed the mistakes then we will publish it.
... provide your conditions for publishing the report or leave only standard disclosure policy link ...
Standard disclosure policy. Use the standard one
Contact information (optional)
... Provide information to contact you or the smart contract-developer in case high severity issues will be found ... My telegram @mave1211
... Provide information about the media resources of the project you want us to audit (website/ twitter account/ reddit/ telegram channel/ etc.) ... All the social media details are in this link: https://linktr.ee/traderdao
Platform
... In which network will your contract be deployed? (EOS/TRX/ETC/ETH/CLO/UBQ/something else ) ... ARB