Realtoapps Smart Contract Audit

CertiK10 Oct 2022Files named in the report

CertiK reviewed Realto (Realtoapps Smart Contract Audit) on 10 Oct 2022: 22 findings, 0 critical; 13 marked fixed, 9 open.

No open critical findings

Open the report Open Realto’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings22

Statuses are the security firm’s own, from its report.

Severity	Open	Fixed
Critical	0	0
High	8	0
Medium	0	7
Low	1	6

MediumIncorrect ParametersFixed

The function `require` needs to check that the number of unlocked tokens of `_from` is not less than the number of transfers, not the number of `msg.sender`.

MediumLack of Storage GapFixed

For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Otherwise it may be very difficult to write new implementation code. Without storage gap, the variable in child contract might be overwritten by the upgraded base contract if new variables are added to the base contract. Refer to https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable We noticed that there are several contracts that inherits upgradable contracts, but do not have storage gap.

MediumPotential Arbitrage RiskFixed

The hacker can perform a potential arbitrage with the function `dividendsOwing()`. Example. 1) If A has 100 tokens, and A transfers 100 tokens to B, A can call `withdrawDividendOnchain()` to claim the dividend. 2) B transfers 100 tokens to C, and B can also claim dividend. ... So all the dividends in the contract will be taken by the hacker.

MediumPotential Incorrect ValidationFixed

The function `freezeTokens()` is called by `DEFAULT_ADMIN_ROLE` to freeze the user's tokens. If the user already has some tokens frozen, then the verification of function `require` will not be appropriate.

MediumPotential Logical InconsistencyFixed

1, The function `transferFrom()` in contract `BaseERC20` is invoked to transfer tokens and is not charged, but the function `transfer()` is charged, so the user can avoid the charge by calling the `transferFrom()`. 2, The function `buy()` in contract `CrowdSales` is called to buy tokens using eth and check that whether the user is locked, but the `buyWithStablecoin()` does not check.

MediumWrong Input ValidationFixed

1, Lines 102 and 104 of the code in contract `BaseERC20` should use `_msgSender` instead of `msg.sender`. 2, 115 lines of code for contract `BaseERC20`, the modifier `adminOrMsgSender` parameter should use `_msgSender` instead of `_address`. 3, Is it possible to replace `msg.sender` and `msg.value` with `_msgSender()` and `_msgData()` in the `buy()` function of the contract `CrowdSales`?

MediumWrong Input ValidationFixed

The function `transferFrom()` is called to transfer the tokens from `_from` to `_to`, but does not check if the address `_from` and `_msgSender()` are locked, and the `forceTransfer()` function has the same issue.

LowHardcode AddressesFixed

There are hardcode addresses in this codebase.

LowLack of `_lockId` Existence CheckFixed

The function `_freezeTokens()` is invoked to freeze the user's tokens, but does not check if the `_lockId` already exists, if it exists, then the data will be overwritten and the frozen tokens will not be unlocked.

LowNo empty initializer constructor for UUPS upgradaFixed

It's recommended the team verify the UUPS implementation contract is initialized correctly. Besides initializing the contract, another way to prevent the implementation contract from being used is to invoke the "_disableInitializers" function in the constructor to automatically lock it when it is deployed. For more information, please refer to: https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#initializing_the_implementation_contract

Show 3 more

LowRedundant StatementsFixed

The value of the `currentDividned` is declared as `uint256`, so the variable `currentDividned` is always greater than or equal to `0`. Also, it seems the variable name should be `currentDividend` instead of `currentDividned`

LowUnlocked Compiler VersionFixed

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

LowUnused Imports and FunctionsFixed

1, The contract `Ownable` is imported, but no functions from `Ownable` are used. 2, The internal functions `_msgSender()` and `_msgData()` are declared, but not used. 3, The modifier `canTransfer` is declared, but not used. 4, The value of the `lockTokens` is defined as `uint256`, so the variable `lockTokens[_address]` is always no less than `amount`, which defaults to 0.