AIDAv2
CertiK reviewed AIDAv2 on 9 Jan 2025: 39 findings, 1 critical; 36 marked fixed, 3 open.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Findings39
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 1 |
| High | 0 | 0 | 4 |
| Medium | 0 | 0 | 9 |
| Low | 3 | 0 | 22 |
CriticalUpgrade To New Version Resulted In Storage CollisFixed
The proxy of the contract is at address: [0x0b7836da458c557e62d48460a0ea86a5392aafe5](https://bscscan.com/address/0x0b7836da458c557e62d48460a0ea86a5392aafe5). Based on the responses from the client it was upgraded to the implementation in this repo [https://github.com/aidatify/AIDAV2/blob/main/AIDAV2.sol](https://github.com/aidatify/AIDAV2/blob/main/AIDAV2.sol) in the transaction [0x917dcc3f0835b8d1c76cbb951f1809418a116aa999a8bfdae1845638b0b93c82](https://bscscan.com/tx/0x917dcc3f0835b8d1c76cbb951f1809418a116aa999a8bfdae1845638b0b93c82). Once upgraded to this version the following actions were taken. It should be noted that it seems the storage of the deployed contract does not match the contract in this repo, so a different implementation may have been used. This will vary what storage is affected, however, it has been verified that the upgrade resulted in storage collision and causes significant problems. - `setupMultiSig()` tx hash: 0x928e249caf036a5bc1f5b0ac8c02c2e3a147d210da2812f4bfefca1fcecdcc78 - `setupTimeLock()` tx hash: 0x8be4f80290c835b9b50859b864438a1864bf57f348d2850388e7e57a87f64ac8 - revoke admin role tx hash: 0x1ff4e0689ecf44e97eaed1aa226f7f501ccf6382c0e22bfb02452f87df26375c However, after these actions, the implementation was upgraded in the transaction [0x99657a6bd30a5886e8cb30da0afb24459eec3b08abb1595e96b7b2f3bc5468f1](https://bscscan.com/tx/0x99657a6bd30a5886e8cb30da0afb24459eec3b08abb1595e96b7b2f3bc5468f1) to have the implementation at address [0xAB552D1Fae8bD3C7e3823130d98Be31BBb7C2BA5](https://bscscan.com/address/0xAB552D1Fae8bD3C7e3823130d98Be31BBb7C2BA5), which caused storage collision. This causes issues with the storage for `nonces`, `isOwner`, and `lastOperationTime` mappings may store and reference at different storage addresses so that it no longer references the same storage. The variables `ownerCount`, `threshold`, `nonce`, `nameHash`, and `versionHash` reference different storage slots. As such they reference values they were not originally set to. The array `owners` also stores and references at different storage addresses, so that it no longer references the same storage. Calls to `checkSignatures()` will all fail. This is because `threshold` references storage slot 16. In the previous version storage slot 16 referenced `nonce`, which was set to be 1 when `upgradeToAndCall()` was called to upgrade to the new implementation. Thus the current `threshold` value is 1 (note that if this was set to a value of 0 anyone could have upgraded the proxy to reference a malicious implementation). Since the threshold is 1, there must be at least one signature that can be counted as valid. However, to be counted as valid the recovered signer must be true in the `isOwner` mapping. However, as this references a new storage address this will be false for all of the owners set via the `setupMultiSig()` call and prevent any signature provided from being counted as valid. As a result, all functions that call `checkSignatures()` will not be able to be successfully called. In particular, the contract cannot be upgraded to fix the storage issues.
High`checkSignatures()` Returns True Prior To `setupMFixed
The function `checkSignatures()` does not make any checks to ensure that the `threshold > 0`. If the `threshold = 0`, then an empty signature can be input with any message hash and it will return true. `_setupOwners()` ensures that the threshold will be set to be greater than or equal to 1, however, this is only called via the function `setupMultiSig()` which can only be called after initialization. Thus an attacker can bypass any function whose access restriction is dependent on `checkSignatures()` until `setupMultiSig()` is called. In particular, before `setupMultiSig()` is called an attacker can do the following: - Upgrade the implementation of the proxy. If this goes unnoticed and it is upgraded to a malicious implementation, all funds could be stolen. - Set the receivers, aidReceiver, and signer to addresses they control. - Withdraw funds when they did not have any funds staked, stealing them from the protocol. - Stake when they did not supply any funds, enabling them to then later unstake the funds, stealing the amount from the protocol.
High`unstake()` can be replayedFixed
```sol=243 function unstake(bytes calldata signature, uint256 stakeId, uint256 _amount, uint256 deadline) external { ``` `unstake()` allows to be called with the same arguments many times. If the `signer` has produced the signature for the message with `DOMAIN_SEPARATOR(), "unstake", stakeId, _amount, deadline`, and the `_amount` allowed to be unstaked is lower than `stakeRecords[stakeId].amount`, then the same user can call the function for the second time and unstake the same `_amount` for the second time, which was not supposed by the `signer`.
HighCentralization Risks in aidav2.solFixed
In the contract `AiDatify`, the role `DEFAULT_ADMIN_ROLE` has authority over the functions shown in the diagram below. Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow the hacker to take advantage of this authority and set the aid receiver to misdirect funds to a hacker-controlled address, set receivers, and their receive rates to misdirect funds, upgrade the contract implementation, set the signer address to make all previous signature invalid and forge new signatures for unauthorized sensitive actions.  ---- In the contract `AiDatify`, the role `signer` has authority over the following functions: - `unstake()`; - `withdraw()`; - `swap()`; Any compromise to the `signer` account may allow the hacker to take advantage of this authority and forge valid signatures to unstake, withdraw, or swap contract tokens allowing to misdirect funds to a hacker-controlled address. In addition, the `signer` can withhold providing signatures resulting in users being unable to recover their funds. Furthermore, the contracts interact with many third parties and out of scope contracts which hold and control the funds. As such, the in scope contracts do not guarantee that there will be enough funds for a user to recover their expected amount.
HighContract Upgrade Centralization RiskFixed
In the contract `AiDatify`, the role `DEFAULT_ADMIN_ROLE` has the authority to update the implementation contract. Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow a hacker to take advantage of this authority and change the implementation contract therefore executing potential malicious functionality in the implementation contract.
Medium`AiDatify` does not approve the usage of `AID` byFixed
In the `swap()` function, when swapping `AID` for `USDT`, the `AID` tokens are transferred from `AID_SENDER` to `address(this)`, which are then supposed to be transferred by the `ROUTER` through `ROUTER.swapExactTokensForTokensSupportingFeeOnTransferTokens()`. However the `swapExactTokensForTokensSupportingFeeOnTransferTokens()` require approval from `address(this)`. Because of this lack of approval, the swap will fail.
Medium`initialize()` Is UnprotectedFixed
The `AiDatify` logic contract does not protect the initializer. An attacker can front-run the `initialize` call and assume ownership of the logic contract. Once in control, the attacker can perform privileged operations, misleading users into believing that they are interacting with the legitimate owner of the upgradeable contract.
MediumIncorrect `stakeIds` Updated in `stakeUSDT()`Fixed
The function `stakeUSDT()` updates `stakeIds` as follows: ```solidity=348 stakeRecords[stakeId] = StakeRecord({amount: amount, owner: addr, status: false}); stakeIds[msg.sender].add(stakeId); ``` instead of ```solidity stakeRecords[stakeId] = StakeRecord({amount: amount, owner: addr, status: false}); stakeIds[addr].add(stakeId); ```
MediumInvalid Timelock MechanismFixed
The current timelock mechanism does not correctly enforce delayed execution for privileged functions. Instead of requiring a specific waiting period before executing functions, it only restricts calling the function again within a set interval. This approach fails to provide adequate security. For instance, in a valid timelock, `upgradeToAndCall()` would require a request and a delay before execution, ensuring users can assess the operation's safety and respond as needed.
MediumMissing Input Validation in `setReceivers()`Fixed
The `setReceivers()` function does not validate inputs adequately, allowing for duplicated addresses in the `receivers` array and zero values in the `receiverRate` array potentially leading to unexpected behavior.
Show 10 more
MediumNo Method To Add/Remove OwnersFixed
The `owners` array holds the addresses who can provide signatures to enable the use of functions that utilize `checkSignatures()`. In effect this behaves as a multi-sign which is designed to protect against the compromise of one or more of the owners private keys. However, there is no method to add or remove an owner once the owners are set. If an owners private key is compromised, this means they cannot be removed or replaced with an account whose private key is not compromised. As a result the compromised account can always be used to provide a signature, so that it reduces the protection of the multi-sign as then the attacker needs only to compromise `threshold - 1` more owners private keys.
MediumPotential `chainId` Replay AttackFixed
The `chainId` is hardcoded as `CHAIN_ID = 56` in the current implementation, which makes the chain-id replay attack possible. For example, when a fork occurs, the new chain will have a different `chainId` than that of the current chain. The message uses the `chainId` variable to prevent it from being replayed on both the main chain and the side chain, however, as the `chainId` is constant, the contracts of the main chain and the forked chain will have the same message thus allowing the attacker to use these messages on both chains whereas it should have only been used on one chain.
MediumPotential Replay AttackFixed
The functions `unstake()`, `swap()` and `withdraw()` do not use `DOMAIN_SEPARATOR()` in their signature verification. This could allow a replay attack by using a message signed for another instance of contract, on another chain, or a testnet. It is also possible for a user to use a valid signature for `swap()` for the function `withdraw()` since they have the same parameter types and are encoded similarly.
MediumPotential Sandwich AttacksFixed
The function `swap()` derives the minimum output amount by calling the `getAmountsOut()` function. This will give the exact amount of tokens that will be received provided no fees are taken on transfer. Note that AID is a fee on transfer token that currently takes a 3% fee on sell and a 6% fee on buy, so that when selling via this function the actual amount of tokens out is given by `swapOut = getAmountsOut((srcAmount * 970) / 1000, path)` and when buying the actual amount of tokens out is given by `getAmountsOut(srcAmount, path) * 940 / 1000`. When selling the current minimum amount is set to be `(getAmountsOut((srcAmount * 970) / 1000, path) * 90) / 100` which will always hold as it is 90% of the actual amount that will be received. When buying it uses `swapExactTokensForTokens()` as opposed to `swapExactTokensForTokensSupportingFeeOnTransferTokens()` so that the minimum amount is checked against the calculated amount and not necessarily the amount received due to fees. In this case it will also always hold as `(getAmountsOut(srcAmount, path) * 90) / 100` is 90% of the calculated amount. With the minimums currently used it is not possible to control the slippage and allows for sandwich attacks. In particular, swaps can happen at a manipulated price and end up receiving fewer tokens than current market price dictates.
Low`aidStake` is never usedFixed
The state variable `aidStake`: ```solidity=48 address public aidStake; ``` is defined but never used.
Low`OPERATOR_ROLE` never usedFixed
The `OPERATOR_ROLE` is defined and granted to the initializer, however, this role is never used for controlling access to specific functions.
LowApproval from `AID_SENDER`Fixed
`swap()` and `withdraw()` rely on a `transferFrom()` of `AID` from an external address. This requires that `AiDatify` is always approved by the `AID_SENDER` address to perform the transfer. If the approval is not made `swap()` and `withdraw()` will fail.
LowBlock Timestamp Used As Deadline in swapFixed
When swapping tokens, a deadline is typically set to prevent transactions from being executed after a certain amount of time, which could result in unfavorable price slippage. However, in the current contract, when conducting transactions, the deadline is set to `block.timestamp` meaning "the deadline check will pass whatever block the transaction is in". If bots and maximal extractable value (MEV) strategies are widely used, it could affect many transactions. A malicious miner can hold the transaction as he wants to incur the maximum loss is reached. Therefore, the absence or incorrect usage of a deadline could make transactions more susceptible to front-running / MEV attacks, leading to greater price slippage losses.
LowBuying Tokens Via `swap()` Does Not Utilize FunctFixed
When buying `AID` tokens via `swap()` the logic utilizes `swapExactTokensForTokens()` as opposed to `swapExactTokensForTokensSupportingFeeOnTransferTokens()`. This will cause the minimum amount check to not account for the actual amount of tokens received and may result in more slippage than desired to be allowed during the swap.
LowCritical Library Functions Skipped SilentlyFixed
`upgradeToAndCall()`, `grantRole()`, and `revokeRole()` functions are overridden and emptied allowing sensitive operations to be triggered by alternative functions. Leaving these overridden functions empty can lead to confusion and unintended silent failures leading to misinterpretations of the contract's behavior.
Show 10 more
LowExternal Call Inside LoopFixed
External calls are made inside a `for` loop. This might lead to a denial-of-service attack. If any of the calls fail, it will cause the entire loop to revert. ```solidity=234 IStakePool(STAKE_POOL).redeemUnderlying(_amount); ``` ```solidity=236 ERC20Upgradeable(USDT).transfer(FEE_RECEIVER, fee); ``` ```solidity=237 ERC20Upgradeable(USDT).transfer(msg.sender, _amount - fee); ```
LowInefficient Memory ParameterFixed
The `_owners` parameter with `memory` data location is never modified in the function `initialize()` which is never called internally within the contract. Thus, the data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory. ```solidity=88 function initialize(address[] memory _owners, uint256 _threshold) public initializer { ``` `initialize` has memory location parameters: `_owners`.
LowInherited Contracts Not Initialized In InitializeFixed
In the new implementation, contract `AiDatify` extends `AccessControlUpgradeable`, but the extended contract is not initialized by the current contract. Generally, the initializer function of a contract should always call all the initializer functions of the contracts that it extends.
LowInvalid parameter in `StakeLog` eventFixed
The function `stakeUSDT()` emits the following event: ```solidity=350 emit StakeLog(addr, amount, stakeId, 0, orderId); ``` However, the last parameter should be `block.timestamp` based on the declaration of `StakeLog`: ```solidity=83 event StakeLog(address indexed member, uint256 indexed amount, uint256 stakeId, uint256 operation, uint256 timestamp); ```
LowLack of Input Validation on `token` in `withdraw(Fixed
The `withdraw()` is expected to handle only `AID` and `USDT` tokens. However, due to the `if else if` pattern used it is possible to have another token as a parameter without the function catching it, potentially causing the emission of an invalid event and an `orderId` set as `true`.
LowMagic Numbers for Minimal Stake AmountFixed
The contract sets the minimum stake amount using a hardcoded value, assuming USDT's decimal configuration on Binance Smart Chain (BSC), where USDT has 18 decimals. However, if the contract is ever deployed on another chain, such as Ethereum, where USDT has only 6 decimals, the same minimum stake amount would not be reliable or appropriate. This use of a "magic number" without clear context or configurability introduces potential inconsistencies if the contract is deployed across different chains with varying USDT decimal settings.
LowMissing Check on Signer Address InitializationFixed
The functions verifying a signature compare the decoded address against the address set as the `signer`. However, there is no check to ensure that the signer address has been initialized or set before these comparisons occur. This lack of initialization checks can lead to errors where supposedly valid signatures fail.
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowNon-Recommended Deletion Method of `EnumerableSetFixed
The contract uses `delete stakeIds[msg.sender]` to clear the user's set of staked IDs stored in an `EnumerableSet.UintSet` when unstaking. \ The `delete` operation resets the set reference but does not properly clear the internal data (`_values` and `_indexes`). OpenZeppelin’s documentation advises against using `delete` on `EnumerableSet` types, as it can leave internal data in a corrupted or misleading state: > Trying to delete such a structure from storage will likely result in data corruption, rendering the structure unusable. See ethereum/solidity#11843 for more info. \ > In order to clean an EnumerableSet, you can either remove all elements one by one or create a fresh instance using an array of EnumerableSet. — [**OpenZeppelin Contracts Documentation**](https://docs.openzeppelin.com/contracts/5.x/api/utils#EnumerableSet).
LowRedundant StatementFixed
In `unstake()` function, in the following block of code: ```solidity=256 if (_amount == amount) { record.status = true; if (stakeIds[msg.sender].contains(stakeId)) { stakeIds[msg.sender].remove(stakeId); } } ``` the line `if (stakeIds[msg.sender].contains(stakeId)) {` is redundant, indeed the `remove()` function only returns false if `stakeId` is not contained.
Show 6 more
LowRisk of Stuck ETHFixed
`aidav2` contract has a `receive()` and payable functions allowing it to accept incoming ETH, but it does not include any mechanisms for managing or retrieving the received ETH. \ This creates a significant risk of ETH becoming permanently stuck within the contract, as there are no implemented functions to facilitate outgoing transfers, withdrawals, or refunds
LowRisk of Token Dust Due to Rounding Errors in AlloFixed
The distribution mechanism in the contract utilizes the formula `ERC20Upgradeable(USDT).transfer(receivers[i], (amount * receiveRates[i]) / 100);` to allocate tokens among multiple receivers. This method can lead to token dust, where a small amount of tokens remains undistributed due to the nature of integer division. Such scenarios occur when the product of the total amount and the percentage does not result in a whole number, and the division truncates any fractional remainder.
LowUnchecked Value of ERC-20 `transfer()`/`transferFFixed
The return values of the `transfer()` and `transferFrom()` calls in the smart contract are not checked. Some ERC-20 tokens' transfer functions return no values, while others return a bool value, they should be handled with care. If a function returns `false` instead of reverting upon failure, an unchecked failed transfer could be mistakenly considered successful in the contract. ```solidity=283 ERC20Upgradeable(token).transferFrom(msg.sender, aidReceiver, amount); ``` ```solidity=264 ERC20Upgradeable(USDT).transfer(FEE_RECEIVER, fee); ``` ```solidity=297 ERC20Upgradeable(AID).transferFrom(AID_SENDER, to, amount); ``` ```solidity=320 ERC20Upgradeable(AID).transferFrom(AID_SENDER, address(this), srcAmount); ``` ```solidity=236 ERC20Upgradeable(USDT).transfer(FEE_RECEIVER, fee); ``` ```solidity=237 ERC20Upgradeable(USDT).transfer(msg.sender, _amount - fee); ``` ```solidity=216 ERC20Upgradeable(USDT).transfer(receivers[i], (amount * receiveRates[i]) / 100); ``` ```solidity=265 ERC20Upgradeable(USDT).transfer(msg.sender, _amount - fee); ``` ```solidity=300 ERC20Upgradeable(USDT).transfer(to, amount); ``` ```solidity=324 ERC20Upgradeable(USDT).transferFrom(msg.sender, address(this), srcAmount); ``` ```solidity=206 ERC20Upgradeable(USDT).transferFrom(msg.sender, address(this), amount); ```
LowUnnecessary Import of `ERC20BurnableUpgradeable`Fixed
The contract imports `@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20BurnableUpgradeable.sol`, which adds burnable functionality to an ERC20 token, allowing tokens to be irreversibly destroyed. However, this additional feature is not utilized within the contract, making the import redundant.
LowUnused Return ValueFixed
The smart contract does not check or store the return value of an external call in a local or state variable, which may introduce vulnerabilities due to the unhandled outcome. ```solidity=329 ROUTER.swapExactTokensForTokens(srcAmount, (swapOut * 90) / 100, path, aidReceiver, block.timestamp); ``` ```solidity=96 ERC20Upgradeable(USDT).approve(address(ROUTER), type(uint256).max); ``` ```solidity=209 stakeIds[msg.sender].add(stakeId); ``` ```solidity=259 stakeIds[msg.sender].remove(stakeId); ``` ```solidity=349 stakeIds[msg.sender].add(stakeId); ```
LowUse of `ERC20Upgradeable` for USDTFixed
The contract uses `ERC20Upgradeable` to interact with USDT, which is not fully compliant with the `ERC20Upgradeable` standard.
Contracts in scope
No deployed address is recorded for this audit.
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (1)
- contracts/aidav2.sol
Part of AIDAv2’s security record
Security firm
CertiK1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us