Ease
Smart Contract Security Assessment
July 20, 2022
SUMMARY
ABSTRACT
Dedaub was commissioned to perform a security audit of parts of the Ease protocol.
The Ease protocol is a DeFi insurance protocol that uses the assets being covered as the underwriting funds for each other, thus making outside underwriters completely unneeded. This audit report covers the contracts listed below from the following repository EaseDeFi/gvToken of the Ease protocol, up to commit hash 9379907aaea908c0cc762310f90e16b95f2ab1ab. Two senior auditors along with one junior auditor worked over the codebase for over 6 days.
The audited contract list is the following:
contracts/core/BribePot.sol
contracts/core/EaseToken.sol
contracts/core/GvToken.sol
contracts/core/TokenSwap.sol
contracts/external/SolmateERC20.sol
contracts/interfaces/IBribePot.sol
contracts/interfaces/IEaseToken.sol
contracts/interfaces/IERC20.sol
contracts/interfaces/IGvToken.sol
contracts/interfaces/IRcaController.sol
contracts/interfaces/IVArmor.sol
contracts/library/MerkleProof.sol
The codebase appears to be well-tested and covers many corner cases. A big part of the codebase is sufficiently documented. Supplementary documentation on the GvToken, the BribePot, their interactions and the roles of the users of the protocol was also provided. One high severity issue was identified, which if not addressed could lead to users not being able to retrieve their deposits in whole from the GvToken contract. This issue can be easily resolved by the protocol team.
SETTING & CAVEATS
The audit’s main target is security threats, i.e., what the community understanding would likely call "hacking", rather than regular use of the protocol. Functional correctness (i.e., issues in "regular use") is a secondary consideration. Typically it can only be covered if we are provided with unambiguous (i.e., full-detail) specifications of what is the expected, correct behavior. In terms of functional correctness, we often trusted the code’s calculations and interactions, in the absence of any other specification.
Functional correctness relative to low-level calculations (including units, scaling, quantities returned from external protocols) is generally most effectively done through thorough testing rather than human auditing. The scope of the audit includes smart contract code. Interactions with off-chain (front-end or back-end) code are not examined other than to consider entry points for the contracts, i.e., calls into a smart contract that may disrupt the contract’s functioning.
VULNERABILITIES & FUNCTIONAL ISSUES
This section details issues that affect the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:
- User or system funds can be lost when third-party systems misbehave.
- DoS, under specific conditions.
- Part of the functionality becomes unusable due to a programming error.
- Breaking important system invariants but without apparent consequences.
- Buggy functionality for trusted users where a workaround exists.
- Security issues which may manifest when the system evolves.
Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.
CRITICAL SEVERITY
[No critical severity issues]
HIGH SEVERITY
H1
A user depositing in the GvToken contract might not be able to retrieve their deposit in whole under certain circumstances
When a user deposits EASE tokens in the GvToken contract, a deposit receipt is added to the _deposits array, which holds one receipt for every deposit that the user has made. When the user decides to withdraw their funds (in whole or partially) they can call the function withdrawRequest, which computes how many of the latest deposits need to be considered to match the withdrawal amount and stores that number N. After a set period of time (e.g. 2 weeks), the user is able to call withdrawFinalize which will transfer to them the deposited amount of EASE tokens that had been requested and will remove the last N deposits from the _deposits array.
The deposit, withdrawRequest and withdrawFinalize functions will operate as expected in isolation. However, let’s consider the following scenario, which consists of the following sequence of actions [deposit -> withdrawRequest -> deposit -> withdrawFinalize] while tracking the state of the GvToken:
-
A user makes a deposit of 100 EASE:
-
_totalDeposit[user] = 100
-
_deposits[user] = [100]
-
N = 0
-
The user decides to withdraw 10 of those:
-
_totalDeposit[user] = 100
-
_deposits[user] = [90, 10]
-
N = 1
-
Before finalizing the withdrawal request (2 weeks delay) they change their mind and decide to deposit another 100:
-
_totalDeposit[user] = 200
-
_deposits[user] = [90, 10, 100]
-
N = 1
-
After that they finalize their initial withdrawal request, which pops the last one deposit, expecting it's the 10 EASE:
-
_totalDeposit[user] = 190
-
_deposits[user] = [90, 10]
-
N = 0
After this sequence of actions the user will be able to withdraw at most 100 EASE even though _totalDeposit[user] is 190. This is because if withdrawRequest is called execution will reach function _updateDepositsAndGetPopCount and as the condition totalAmount >= withDrawAmount in the for loop will never be true (totalAmount will be strictly less than withDrawAmount), execution will not break out of the loop. Loop variable i will become 0 at the end of the loop and the last line of the function, index - (i - 1), will cause an underflow. Thus, the user will not be able to ever withdraw their deposit in whole.
MEDIUM SEVERITY
[No medium severity issues]
LOW SEVERITY
There are a few TODOs in different parts of the code. All of them are of minor importance, but still we would advise in favor of resolving them before the final review of all the audits fixes is done by the auditing team.
Below is a list with functions containing TODO items:
- BribePot::bribe
- BribePot::_getBribeUpdates
- GvToken::withdrawFinalize
- GvToken::stake
- GvToken::unstake
- GvToken::setDelay
- TokenSwap::swapVArmor
Not all calls to ERC20 tokens are using the SafeERC20 wrappers. For the EASE token, which extends SolmateERC20, this would not make a difference as SolmateERC20 functions always return true. Nevertheless, it is still advised that SafeERC20 wrappers are used everywhere, as in a future version of the codebase SolmateERC20 might be dropped for a different ERC20 implementation that exhibits different behavior.
CENTRALIZATION ISSUES
It is often desirable for DeFi protocols to assume no trust in a central authority, including the protocol’s owner. Even if the owner is reputable, users are more likely to engage with a protocol that guarantees no catastrophic failure even in the case the owner gets hacked/compromised. We list issues of this kind below. (These issues should be considered in the context of usage/deployment, as they are not uncommon. Several high-profile, high-value protocols have significant centralization threats.)
No centralization issues were identified. There is only a single TODO comment in function TokenSwap::swapVArmor that mentions the possibility of fixing the conversion rate of vArmor to Armor token according to a certain period in time in order to avoid calling function vArmorToArmor of the vArmor token and save gas. If this is implemented and the rate is decided solely by the protocol owners, it could prove to be a centralization point.
OTHER/ ADVISORY ISSUES
This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.
Function _update of the BribePot contract calls BribePot::_getBribeUpdates and BribePot::_rewardPerToken to update storage variables lastBribeUpdate, bribePerWeek and lastRewardUpdate and to compute the latest reward per token . After that BribePot::earned is called, which again calls BribePot::_getBribeUpdates and BribePot::_rewardPerToken to get the per token reward in order to compute the amount of EASE token earned for bribing gvEASE .
function earned(address account) public view returns (uint256) {
(
uint256 additionalRewardPerToken,
uint256 currBribePerWeek,
) = _getBribeUpdates();
uint256 currRewardPerToken = _rewardPerToken(
additionalRewardPerToken,
currBribePerWeek
);
return
((_balances[account] *
(currRewardPerToken - (userRewardPerTokenPaid[account]))) /
(MULTIPLIER)) + rewards[account];
}
The second call to BribePot::_getBribeUpdates and BribePot::_rewardPerToken are unnecessary. Creating a stripped version of earned, _earned, and calling that at the end of _update would result in gas savings.
function _earned(
address account,
uint256 rewardPerToken
) private view returns (uint256) {
return
((_balances[account] *
(rewardPerToken - (userRewardPerTokenPaid[account]))) /
(MULTIPLIER)) + rewards[account];
}
Functions withdraw and getReward of the BribePot contract are defined as public even though they are not called by any BribePot function and thus could be made external.
Function GvToken::_deposit should disallow deposits when the total deposited amount is 0, i.e., parameters amount and rewardAmount are both 0. Otherwise, a user will unnecessarily spend funds in gas when depositing 0 EASE by mistake and will also make their future interactions with the contract more expensive for no reason as they will be adding a zero value deposit in the _deposits array.
Storage variables GvToken::gov and BribePot::gvToken can be made immutable.
There are a couple incorrect comments:
-
Comment about the GvToken::_stakes storage variable
/// user => rcaVault => % of gvEASE
/// Dedaub: rcaVault => user => % of gvEASE
mapping(address => mapping(address => uint256)) private _stakes; -
Comment in GvToken::_updateDepositsAndGetPopCount
// If there is a remainder we need to update the index at which
// we broke out of loop and push the withdrawan amount to user
// _deposits withdraw 100 ease from [75, 30] EASE balance becomes
// [5, 70, 75]
// Dedaub: [5, 30, 70]
if (remainder.amount != 0) {
Deposit memory lastDepositWithdrawan = _deposits[user][i - 1];
lastDepositWithdrawan.amount -= remainder.amount;
_deposits[user][i - 1] = remainder;
_deposits[user].push(lastDepositWithdrawan);
}
Address 0xdEaD used in TokenSwap can be defined as a contract constant.
The floating version pragma solidity ^0.8.11 allows contracts to be compiled with any version of the Solidity compiler ranging from 0.8.11 to 0.9.0. Even though versions might not differ drastically, floating pragmas should be avoided and the pragma should be fixed to the version that will be used for the contracts’ deployment.
The contracts can be compiled with any version of the Solidity compiler ranging from 0.8.11 to 0.9.0, which, at the time of writing, have some known bugs. We inspected the bugs listed for this version and concluded that the subject code is unaffected.
DISCLAIMER
The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.
ABOUT DEDAUB
Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub's auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.