Avalaunch Audit Allocation Staking and Sales
Introduction
CoinFabrik was asked to audit the contracts for the Avalaunch project. First we will provide a summary of our discoveries and then we will show the details of our findings.
Summary
The contracts audited are from the Github repository at
https://github.com/avalaunch-app/xava-protocol/. The audit is based on the commit fd252f8b9b0283d245d9d561130fe789ff08dfe9. Next, developers fixed issues and we re-checked commit ac00d9c0d66b4abc3a892530c9dfc349811050ad.
Contracts
The audited contracts are:
? contracts/AllocationStaking.sol
? contracts/sales/SalesFactory.sol
? contracts/sales/AvalaunchSale.sol
Analyses
The following analyses were performed:
? Misuse of the different call methods
? Integer overflow errors
? Division by zero errors
? Outdated version of Solidity compiler
? Front running attacks
? Reentrancy attacks
? Misuse of block timestamps
? Softlock denial of service attacks
? Functions with excessive gas cost
? Missing or misused function qualifiers
? Needlessly complex code and contract interactions
? Poor or nonexistent error handling
? Failure to use a withdrawal pattern
? Insufficient validation of the input parameters
? Incorrect handling of cryptographic signatures
Findings and Fixes
Severity Classification
Security risks are classified as follows:
? Critical: These are issues that we manage to exploit. They compromise the system seriously. They must be fixed immediately.
? Medium: These are potentially exploitable issues. Even though we did not
manage to exploit them or their impact is not clear, they might represent a
security risk in the near future. We suggest fixing them as soon as possible.
? Minor: These issues represent problems that are relatively small or difficult to take advantage of but can be exploited in combination with other issues. These kinds of issues do not block deployments in production environments. They should be taken into account and be fixed when possible.
? Enhancement: These kinds of findings do not represent a security risk. They are best practices that we suggest to implement.
This classification is summarized in the following table:
Issues Found by Severity
Critical severity
CR-01 Double Withdrawals Allowed in withdrawTokens()
The function fails to mark a portion as withdrawn. This may be due to a typo in the line
p.isPortionWithdrawn[portionId];
As a result, any user can withdraw the same portion more than once.
Recommendation
Set
p.isPortionWithdrawn[portionId] = true;
Solution
The issue was fixed following the recommendation.
CR-02 Earlier-than-expected Withdraw in withdrawTokens()
When withdrawing tokens for a given portionID the function checks that
vestingPortionsUnlockTime[portionId] >= block.timestamp
so that all portions can be vested at the start.
Recommendation
Check the opposite condition.
Solution
The issue was fixed following the recommendation.
Medium severity
ME-01 Denial of Service in depositTokens()
The function depositTokens() in AvalaunchSale requires that
sale.token.balanceOf(address(this)) == 0
Hence, a malicious user could transfer a minimal amount of tokens to the contract so the above condition is not met.
Recommendation
Instead, use
require(!sale.tokensDeposited, message);
Solution
The issue was fixed following the recommendation
ME-02 Excessive DepositFees Through Settings Manipulation
Both when the AllocationStaking contract is initialized and when setDepositFee() is called, _depositFeePercent may be set to any value independently of _depositFeePrecision. The same consideration should apply during initialization (_depositFeePercent must be smaller than the public variable depositFeePrecision = 10e8).
Solution
The issue was fixed following the recommendation.
ME-03 Vesting Settings Allowing Excessive or Insufficient Token Withdrawal
In setVestingParams() no checks are made that the sum of _percents is 100. For example, if _percents[0] = 200, when a user calls withdrawTokens() he would receive twice the amount bought:
p.amountBought.mul(vestingPercentPerPortion[portionId]).div(10 0)
Similarly, if the sum is smaller than 100, the user would be prevented from vesting all the tokens he bought.
Recommendation
Require that the sum of the percentages is 100.
Solution
The issue was fixed following the recommendation.
Minor Severity
MI-01 Denial of Service in postponeSale()
Using a large value for timeToShift in postponeSale() may shiftround.startTime to be bigger than sale.saleEnd rendering the sale useless.
Recommendation
Require that round.startTime + timeToShift < sale.saleEnd.
Solution
The issue was fixed following the recommendation.
MI-02 Requirements Best Practices
In different places in the contacts we see requirements of the following sort:
require(condition == true, msg) require(condition == false, msg)
which although allowed, are not preferred. Instead use require(condition, msg) and require(!condition, msg) respectively. These may use less gas.
Solution
Fixed.
MI-03 Denial Of Service by Removing All the Administrators
The function removeAdmin() can be called arbitrarily and could be used to remove every administrator.
Recommendation
Make sure that this does not happen by requiring that len(Admin) > 1.
Solution
Fixed. The recommended change was applied.
Enhancements
Constants and Code Legibility
Use one in
uint256 amountOfTokensBuying = (msg.value).mul(10**18).div(sale.tokenPriceInAVAX);
instead of 10**18. Similarly, the value 1e36 is used throughout the AllocationStake contract. Consider replacing this with a constant that can be called by its name.
Small Precision In Vesting Scheme
The precision for the percentage points is set to 1 (so that all values go from 1 to 100). Using a larger number could accommodate more flexibility.
Gas Optimization in the Round Structure
The struct Round defined in AvalaunchSale.sol has two unspecified uints (uint startTime; uint maxParticipation;) and in the struct Registration all objects can fit in 256.
Unnecessary Code in setRounds()
The function makes use of a for loop which verifies conditions for every round. However, in some cases, only the first and last rounds need to be checked. Specifically,
require(startTimes[0] > registration.registrationTimeEnds); require(startTimes[0] >= block.timestamp);
since the code also requires that startTimes[i] > startTimes[i-1] for every i. Analogously, checking
require(startTimes[last] < sale.saleEnd);
is sufficient.
Gas Optimization in massUpdatePool() and compound()
Consider factoring updatePoolWithFee() so that the computation of nrOfSeconds is only done once (and not once per pool.) This same behaviour is seen in compound() which makes two calls to updatePoolWithFee() (the first one via updatePool()). In the second, nrOfSeconds will be 0 and most of the logic is useless. Consider
decoupling the two functionalities in order to save gas
Gas Optimization in PoolInfo
A gas optimization for PoolInfo could be done by reducing the size of the
lastRewardTimestamp and allocPoint. For example, fitting each into an uint128 (so the two fit into 32 bytes).
Note that, since 2^32 – 1 is a timestamp for February 2106, 32 bits are enough for a timestamp and 128 is obviously sufficient. If less than 2^128 allocation points are sufficient per pool, then restructuring PoolInfo as we proposed can be done.
Conclusion
We found the contracts to be simple and straightforward. Documentation is scarce. Two critical vulnerabilities were found which could allow a user to withdraw tokens in excess or before their time. These issues were fixed. Three medium severity issues were found, one of which allows a malicious user to make a sale contract unusable, and the two other which can be exercised when the administrator sets incorrect parameters–so they are unlikely to be exploitable. All issues have been resolved.
MORE DETAILS
Disclaimer: This audit report is not a security warranty, investment advice, or an approval of the Avalaunch project since CoinFabrik has not reviewed its platform. Moreover, it does not provide a smart contract code faultlessness guarantee.
El post Avalaunch Audit <>Allocation Staking and Sales apareció primero en CoinFabrik Blog.
Read more: https://blog.coinfabrik.com/avalaunch-audit-allocation-staking-and-sales/
Text source: CoinFabrik Blog
