A time-boxed independent security assessment of the Glifio LiquidityMine's contract was done by Zealynx Security, with a focus on the security aspects of the application’s implementation.
We performed the security assessment based on the agreed scope, following our approach and methodology. During the assessment the contract was reviewed and a report with found issues is shared below. Also, it was created a security test suite implementing Fuzzing and Invariant tests and Formal Verification.
- Audit Report with Fuzzing and Formal Verification Test Suite
- Table of Contents
- Overview
- Vulnerability Findings List
- [M-1] Precision and Rounding Error Vulnerability in Reward Calculations
- [M-2] Imprecision in Rewards Distribution Leading to Residual Dust
- [L-1] Lack of Zero Address Check
- [L-2] Use safeTransfer and safeTransferFrom
- [I-1] Consider adding relevant name to variable
- [I-2] Emit events in Ownable contract
- [I-3] Missing Checks for non-zero transfer value calls
- [I-4] Return values of _computeAccRewards never used
- Security Test List
- Complete Documentation of Test Suite Implemented
- Testing Environments
Below it is shared a list with 2 Medium and 2 Low issues raised and a few Informational suggestions to improve the code quality.Also, it can be found a list and short summary of all the security tests implemented with multiple tools.
Besides that we provided a complete documentation about the test setup, how to run each of the tests and further resources to understand and use the tools used.
Severity | Title | Link | Description |
---|---|---|---|
Medium | Imprecision in Rewards Distribution Leading to Residual Dust | The LiquidityMine contract exhibits imprecision in its rewards distribution mechanism, leading to residual dust left in the contract due to discrepancies between the methods used for calculating accrued rewards and claimed rewards. |
|
Medium | Precision and Rounding Error Vulnerability in Reward Calculations | Precision and rounding errors in reward calculations cause inconsistencies between accRewardsTotal and the sum of claimed and unclaimed rewards. Use higher precision data types or fixed-point arithmetic to ensure consistent calculations. |
|
Low | Lack of Zero Address Check | Lack of zero address validation in LiquidityMine.sol functions: deposit , _withdraw , _harvest , and the constructor. Add zero address validation. |
|
Low | Use safeTransfer and safeTransferFrom | Tokens not compliant with the ERC20 specification could return false from the transfer and transferFrom functions. Use SafeERC20 library from OpenZeppelin and call safeTransfer or safeTransferFrom when transferring ERC20 tokens. |
|
Informational | Consider adding relevant name to variable | Link | Since u is not descriptive, consider giving a descriptive name such as userInfo . |
Informational | Emit events in Ownable contract | Link | Missing events: OwnershipTransferred and OwnershipPending . |
Informational | Missing Checks for non-zero transfer value calls | Checking for non-zero values can prevent an external call to save gas. Functions deposit and _withdraw lack input validation for uint256 amount . Add zero amount validation. |
|
Informational | Return values of _computeAccRewards never used | Out of the three parameters from the return of the function _computeAccRewards , only lockTokenSupply is actually assigned/used. Use named returned parameters or assign the variables to the result to return or remove the named return parameters. |
During the audit of the LiquidityMine
contract, a significant discrepancy was discovered between the total rewards accrued (accRewardsTotal
) and the sum of claimed (rewardTokensClaimed
) and unclaimed rewards. This discrepancy, found during fuzz testing with both high and low values, stems from precision and rounding errors within the contract's reward calculation mechanisms. The magnitude of the discrepancy increases with larger values, indicating a vulnerability in handling large numbers accurately.
The LiquidityMine
contract is designed to distribute rewards to users who lock tokens. However, precision and rounding errors in reward calculations cause inconsistencies between accRewardsTotal
and the sum of rewardTokensClaimed
and unclaimed rewards. These errors occur primarily in the following functions:
updateAccounting
pendingRewards
withdraw
harvest
The key issue arises from the way rewards per token are calculated and updated, specifically in how fixed-point arithmetic is handled in these functions. When large values are involved, the precision loss becomes significant, leading to discrepancies.
The discrepancy results in incorrect reward distributions, causing users to receive less or more than their entitled rewards. This inaccuracy undermines the reward system's integrity and can lead to financial discrepancies and potential disputes among users. The impact is particularly severe with higher values, but even small values exhibit minor discrepancies that undermine the system's reliability.
The following test case demonstrates the discrepancy using fuzz testing with both low and extremely high values, highlighting the difference between the total rewards accrued and the sum of claimed and unclaimed rewards.
function testFuzz_InvariantDepositWithdraw(uint256 depositAmount, uint256 withdrawAmount, address beneficiary) public {
// Limit the fuzzing range for extremely high values
depositAmount = bound(depositAmount, 1e35, 1e40); // Limit deposit amount between 1e35 and 1e40
withdrawAmount = bound(withdrawAmount, 1e35, depositAmount); // Limit withdraw amount between 1e35 and depositAmount
vm.assume(beneficiary != address(0) && beneficiary != address(lm)); // Assume beneficiary is not the zero address or the contract address
// Mint tokens to the beneficiary so they can be deposited
MintBurnERC20(address(lockToken)).mint(beneficiary, depositAmount);
vm.prank(beneficiary);
lockToken.approve(address(lm), depositAmount);
// Perform the deposit
vm.prank(beneficiary);
lm.deposit(depositAmount, beneficiary);
// Add user to the list of users if not already tracked
if (!isUserTracked(beneficiary)) {
users.push(beneficiary);
}
// Load rewards into the contract
uint256 totalRewards = 1e40; // Using an extremely high value for rewards
_loadRewards(totalRewards);
// Simulate passage of time to accumulate rewards
uint256 blocksPassed = 1000;
vm.roll(block.number + blocksPassed);
// Update accounting to reflect the passage of time and accumulated rewards
lm.updateAccounting();
// Ensure there are rewards to harvest
uint256 pendingRewardsBeforeWithdraw = lm.pendingRewards(beneficiary);
console.log("Pending rewards before withdrawal:", pendingRewardsBeforeWithdraw);
// Verify user state before withdraw
LiquidityMine.UserInfo memory userBeforeWithdraw = lm.userInfo(beneficiary);
// Perform the withdraw
vm.prank(beneficiary);
lm.withdraw(withdrawAmount, beneficiary);
// Ensure the rewards are still correct after withdrawal
uint256 pendingRewardsAfterWithdraw = lm.pendingRewards(beneficiary);
console.log("Pending rewards after withdrawal:", pendingRewardsAfterWithdraw);
// Verify user state after withdraw
LiquidityMine.UserInfo memory userAfterWithdraw = lm.userInfo(beneficiary);
// Perform the harvest
vm.prank(beneficiary);
lm.harvest(pendingRewardsAfterWithdraw, beneficiary);
// Verify user state after harvest
LiquidityMine.UserInfo memory userAfterHarvest = lm.userInfo(beneficiary);
// Verify pending rewards after harvest
uint256 pendingRewardsAfterHarvest = lm.pendingRewards(beneficiary);
console.log("Pending rewards after harvest:", pendingRewardsAfterHarvest);
// Check that the total claimed rewards are correct
uint256 totalClaimedRewardsFromContract = lm.rewardTokensClaimed();
uint256 totalClaimedRewardsCalculated = depositAmount.mulWadDown(lm.accRewardsPerLockToken());
// Final assertions
assertEq(pendingRewardsAfterHarvest, 0, "Pending rewards after harvest should be zero");
assertEq(totalClaimedRewardsFromContract, totalClaimedRewardsCalculated, "Total claimed rewards should be correct");
// Additional final state verification
LiquidityMine.UserInfo memory beneficiaryInfo = lm.userInfo(beneficiary);
// Verify accRewardsTotal and rewardTokensClaimed
uint256 accRewardsTotal = lm.accRewardsTotal();
console.log("accRewardsTotal:", accRewardsTotal);
console.log("rewardTokensClaimed:", totalClaimedRewardsFromContract);
uint256 totalUnclaimedRewards = 0;
for (uint256 i = 0; i < users.length; i++) {
totalUnclaimedRewards += lm.userInfo(users[i]).unclaimedRewards;
}
uint256 sumClaimedAndUnclaimed = totalClaimedRewardsFromContract + totalUnclaimedRewards;
uint256 discrepancy = accRewardsTotal > sumClaimedAndUnclaimed ? accRewardsTotal - sumClaimedAndUnclaimed : sumClaimedAndUnclaimed - accRewardsTotal;
console.log("Discrepancy:", discrepancy);
assertEq(
accRewardsTotal,
sumClaimedAndUnclaimed,
"Total rewards accrued should equal the sum of claimed and unclaimed rewards"
);
}
To be discussed
The LiquidityMine
contract exhibits imprecision in its rewards distribution mechanism, leading to residual dust left in the contract. This occurs because of discrepancies between the methods used for calculating accrued rewards and claimed rewards, especially when the new rewards are less than the total locked tokens.
The LiquidityMine
contract calculates accrued rewards based on the number of blocks passed and the reward per epoch. However, claimed rewards are calculated individually for each user based on their interactions with the contract. This difference in calculation methods can lead to slight inaccuracies, particularly when the rewards per epoch divided by the total locked tokens results in a non-integer value, causing rounding issues. When updateAccounting
is called with new rewards less than the total locked tokens, these rounding issues can result in residual dust remaining in the contract, which users cannot claim.
- Residual Dust: Users may miss out on their full reward entitlement, leading to small amounts of unclaimed rewards remaining in the contract.
- User Trust: The presence of unclaimed rewards can undermine user trust, as they may perceive the reward distribution system to be unfair or inefficient.
To demonstrate this vulnerability, a series of fuzz tests were conducted. The tests involved multiple participants depositing, withdrawing, and harvesting rewards. The results showed discrepancies between the total reward cap and the sum of accrued rewards and remaining balance, indicating residual dust.
function testFuzz_ImprecisionInRewardsDistribution(uint256 depositAmount, uint256 withdrawAmount, uint256 rewardAmount, address beneficiary) public {
uint256 numberOfParticipants = 500;
address[] memory participants = new address[](numberOfParticipants);
uint256 blocksPassed = 1000;
// Limit the fuzzing range for more reasonable values
depositAmount = bound(depositAmount, 1 * 1e18, 100 * 1e18);
withdrawAmount = bound(withdrawAmount, 1 * 1e18, depositAmount);
rewardAmount = bound(rewardAmount, 1 * 1e18, 1000 * 1e18);
// Generate and set up participants
for (uint256 i = 0; i < numberOfParticipants; i++) {
participants[i] = vm.addr(i + 1);
vm.assume(participants[i] != address(0) && participants[i] != address(lm));
// Mint lock tokens to the participants
MintBurnERC20(address(lockToken)).mint(participants[i], depositAmount * 2); // Enough for deposit and potential multiple actions
vm.prank(participants[i]);
lockToken.approve(address(lm), depositAmount * 2);
}
// Mint reward tokens to the first participant to load rewards into the contract
MintBurnERC20(address(rewardToken)).mint(participants[0], rewardAmount);
vm.prank(participants[0]);
rewardToken.approve(address(lm), rewardAmount);
// Participants perform deposits
for (uint256 i = 0; i < numberOfParticipants; i++) {
vm.prank(participants[i]);
lm.deposit(depositAmount, participants[i]);
}
// Load rewards into the contract by the first participant
vm.prank(participants[0]);
lm.loadRewards(rewardAmount);
// Simulate passage of time to accumulate rewards
vm.roll(block.number + blocksPassed);
// Update accounting to reflect the passage of time and accumulated rewards
lm.updateAccounting();
// Participants perform withdrawals
for (uint256 i = 0; i < numberOfParticipants; i++) {
vm.prank(participants[i]);
lm.withdraw(withdrawAmount, participants[i]);
}
// Participants harvest their rewards
for (uint256 i = 0; i < numberOfParticipants; i++) {
uint256 pendingRewards = lm.pendingRewards(participants[i]);
if (pendingRewards > 0) {
vm.prank(participants[i]);
lm.harvest(pendingRewards, participants[i]);
}
}
// Validate invariants
uint256 accRewardsTotal = lm.accRewardsTotal();
uint256 totalRewardCap = lm.totalRewardCap();
uint256 rewardTokensClaimed = lm.rewardTokensClaimed();
uint256 remainingBalance = rewardToken.balanceOf(address(lm));
console.log("accRewardsTotal:", accRewardsTotal);
console.log("rewardTokensClaimed:", rewardTokensClaimed);
console.log("remainingBalance:", remainingBalance);
console.log("totalRewardCap:", totalRewardCap);
uint256 calculatedSum = accRewardsTotal + remainingBalance;
assertEq(totalRewardCap, calculatedSum, "totalRewardCap should equal the sum of accRewardsTotal and remaining balance, indicating no dust");
}
To mitigate this issue, consider the following steps:
- Precise Calculation: Implement a more precise calculation method that minimizes rounding errors. For example, using a fixed-point arithmetic library.
- Residual Dust Handling: Introduce a mechanism to periodically sweep residual dust and redistribute it among users or send it to a predefined address. This ensures that users receive their full entitled rewards without any residual amounts left in the contract.
Lack of zero address validation has been found in many instances in
the contract LiquidityMine.sol
. Functions deposit
, _withdraw
, _harvest
, and the constructor are missing this check.
Add zero address validation
Tokens not compliant with the ERC20 specification could return false
from the transfer
and transferFrom
functions call to indicate the transfer fails.
Since the IERC20 token addresses are passed in the constructor, it is convenient to protect from any unexpected implementation.
While the calling contract would not notice the failure if the return value is not checked. Checking the return value is a requirement, as written in the EIP-20 specification:
Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!
Use the SafeERC20
library implementation from OpenZeppelin and call safeTransfer
or safeTransferFrom
when transferring ERC20 tokens.
Since u
is not descriptive, consider giving a descriptive name such as userInfo
UserInfo storage u = _userInfo[user.normalize()];
https://github.com/wadealexc/fevmate/blob/main/contracts/access/OwnedClaimable.sol
event OwnershipTransferred(address indexed oldOwner, address indexed newOwner);
event OwnershipPending(address indexed currentOwner, address indexed pendingOwner);
Severity: Informational
Checking for non-zero values can prevent an external call to save gas. And in deposit
and _withdraw
there is no input validation for uint256 amount
Add:
if (amount == 0) revert ZeroAmountNotAllowed();
Out of the three parameters from the return of the function _computeAccRewards()
only lockTokenSupply
is actually assigned/used.
returns (uint256 newAccRewardsPerLockToken, uint256 newAccRewardsTotal, uint256 lockTokenSupply)
It is recommended to either use named returned parameters without the return
keyword, assigning the variable to the result to return or remove the named return parameters and use the return
keyword.
# | Test Name | Tools Used |
---|---|---|
1 | test_Initialization | Foundry |
2 | testFuzz_Deposit | Foundry & Kontrol |
3 | testFuzz_Withdraw | Foundry |
4 | testFuzz_FinalStateVerification | Foundry & Kontrol |
5 | test_intermediate_rewards | Foundry & Kontrol |
6 | testFuzz_InitialRewardLoadingAndDeposit | Foundry & Kontrol |
7 | testFuzz_MidwayRewardsAndHarvest | Foundry |
8 | testFuzz_LMExtensionAndFinalRewards | Foundry |
9 | testFuzz_InvariantDepositWithdraw | Foundry |
10 | testFuzz_InvariantDepositHighValuesWithdraw | Foundry |
11 | testFuzz_RewardAccumulationAndDistribution | Foundry |
12 | testFuzz_LoadRewards | Foundry |
13 | testFuzz_Harvest | Foundry |
14 | testFuzz_MultiUserDepositHarvest | Foundry |
15 | testFuzz_InvariantPrecisionHighDepositWithdraw | Foundry |
16 | testFuzz_InvariantPrecisionDepositWithdraw | Foundry |
17 | testFuzz_InvariantAccRewardsTotalLessThanOrEqualTotalRewardCap | Foundry |
18 | testFuzz_InvariantMultipleOperations | Foundry |
19 | testFuzz_MultipleUsersOperations | Foundry |
20 | testFuzz_RandomUserActionsWithCheck | Foundry |
21 | testFuzz_ImprecisionInRewardsDistribution | Foundry |
22 | testFuzz_ImprecisionInRewardsDistribution_NormalValues | Foundry |
23 | testFuzz_ImprecisionMultiUser | Foundry |
24 | testNoRewardsAccrue | Foundry |
25 | testNoRewardsAccrue | Foundry |
26 | testFuzz_NoRewardsAccrue | Foundry |
27 | testFuzz_MultipleDepositsTwoUsers | Foundry |
28 | testFuzz_MultipleDepositsSingleUser | Foundry |
29 | test_locked_tokens_increase | Echidna & Medusa |
30 | test_unclaimed_rewards_calculation | Echidna & Medusa |
31 | test_reward_debt_calculation | Echidna & Medusa |
32 | test_deposit_transfer_successful | Echidna & Medusa |
33 | test_locked_tokens_decrease | Echidna & Medusa |
34 | test_unclaimed_rewards_update | Echidna & Medusa |
35 | test_reward_debt_update | Echidna & Medusa |
36 | test_withdraw_transfer_successful | Echidna & Medusa |
37 | test_harvest_rewards_update | Echidna & Medusa |
38 | test_reward_debt_after_harvest | Echidna & Medusa |
39 | test_withdraw_and_harvest_basic | Echidna & Medusa |
40 | test_multiple_deposits_accrual | Echidna & Medusa |
41 | test_reward_cap_exceeded | Echidna & Medusa |
42 | test_set_reward_per_epoch | Echidna & Medusa & Kontrol |
43 | check_Deposit_CheckBalances | Halmos |
44 | check_deposit_positive_amount | Halmos |
45 | check_deposit_updates_reward_debt | Halmos |
46 | check_deposit_more_than_balance | Halmos |
47 | check_updateAccounting_with_locked_tokens | Halmos |
48 | check_updateAccounting_respects_totalRewardCap | Halmos |
49 | check_updateAccounting_invariant | Halmos |
50 | check_withdraw_decreases_locked_tokens | Halmos |
51 | check_harvest_decreases_unclaimed_rewards | Halmos |
52 | check_updateAccounting_no_change_without_new_blocks | Halmos |
53 | check_loadRewards_increases_totalRewardCap | Halmos |
54 | check_withdrawAndHarvest | Halmos |
55 | check_setRewardPerEpoch | Halmos |
56 | check_pendingRewards | Halmos |
57 | check_userInfo | Halmos |
58 | check_pendingRewards_computes_correct_accRewardsPerLockToken | Halmos |
59 | check_fundedEpochsLeft_computes_correct_accRewardsTotal | Halmos |
60 | check_fundedEpochsLeft_ifcomputes_correct_accRewardsTotal | Halmos |
61 | check_rewardsLeft_respects_totalRewardCap | Halmos |
62 | check_updateAccounting_computes_correct_lockTokenSupply | Halmos |
63 | check_initial_deposit_and_intermediate_rewards | Halmos |
64 | check_intermediate_rewards | Halmos |
65 | check_update_accounting_after_initial_deposit | Halmos |
66 | check_second_deposit | Halmos |
67 | check_final_rewards | Halmos |
68 | test_check_deposit_positive_amount | Kontrol |
69 | testcheck_setRewardPerEpoch | Kontrol |
70 | test_pendingRewards_computes_correct_accRewardsPerLockToken | Kontrol |
71 | test_fundedEpochsLeft_computes_correct_accRewardsTotal | Kontrol |
72 | test_rewardsLeft_respects_totalRewardCap | Kontrol |
73 | test_updateAccounting_computes_correct_lockTokenSupply | Kontrol |
74 | testFuzz_InitialDeposit | Foundry & Kontrol |
75 | testFuzz_MidwayPointCheck | Foundry & Kontrol |
76 | testFuzz_FinalRewardsDistribution | Foundry & Kontrol |
77 | invariant_RewardDebtConsistency | Ityfuzz |
78 | invariant_UnclaimedRewardsConsistency | Ityfuzz |
79 | invariant_TotalRewardTokensClaimed | Ityfuzz |
80 | invariant_TotalRewardConsistency | Ityfuzz |
81 | invariant_MaxRewardCapConsistency | Ityfuzz |
82 | invariant_AccRewardsTotalShouldNotExceedTotalRewardCap | Ityfuzz |
83 | invariant_RewardTokensClaimedShouldNotExceedAccRewardsTotal | Ityfuzz |
84 | invariant_assertRewardCapInvariant | Ityfuzz |
85 | invariant_assert_NoResidualDust | Ityfuzz |
Section | Description |
---|---|
Installation | Setup and installation requirements. |
Init | Initial setup and build commands. |
Where to Find the Tests | Locations of different test suites. |
Testing Environments | Overview of testing environments: Foundry, Echidna, Halmos, Ityfuzz, Medusa, and Kontrol. |
Foundry Tests | How to run Foundry tests and where to find them. |
Echidna Tests | Setup and execution of Echidna tests. |
Halmos Tests | Information on setting up and running Halmos tests. |
Ityfuzz Tests | Details on Ityfuzz testing environment and usage. |
Medusa Tests | Instructions for setting up and running Medusa tests. |
Kontrol Tests | Guide to using Kontrol for test execution and debugging. |
To be able to use this repository, you need to have the following installed:
git submodule update --init --recursive
sudo forge build -force
You can find the tests in various folders:
- Foundry in the
test/Fuzz
folder - Echidna in the
src/Echidna
folder - Medusa in the
src/Echidna
folder - Halmos in the
test/FormalVerification
folder - Kontrol in the
test/FormalVerification
folder - Ityfuzz in the
test/Fuzz
folder
- Foundry in the
test/Fuzz
folder
- test/Fuzz/FuzzLiquidityMine.t.sol
forge test --mc FuzzLiquidityMine
forge test --mc FuzzLiquidityMine --mt <test>
- Documentation
- Properties
- echidna
- Echidna Tutorial: #2 Fuzzing with Assertion Testing Mode
- Echidna Tutorial: #1 Introduction to create Invariant tests with Solidity
- Echidna Tutorial: Use Echidna Cheatcodes while Fuzzing
- Echidna in the
src/Echidna
folder
- src/Echidna/EchidnaLiquidityMine.sol
echidna . --contract EchidnaLiquidityMine --config config.yaml
- Medusa in the
src/Echidna
folder
- src/Echidna/EchidnaLiquidityMine.sol
medusa fuzz
- CheatCode
- Documentation
- Formal Verification In Practice: Halmos, Hevm, Certora, and Ityfuzz
- Examples
- Halmos in the test/FormalVerification folder
- test/FormalVerification/HalmosFV.t.sol
halmos --contract HalmosFVLiquidityMine --solver-timeout-assertion 0
halmos --contract HalmosFVLiquidityMine --function <test> --solver-timeout-assertion 0
- GitHub
- Documentation
- Formal Verification In Practice: Halmos, Hevm, Certora, and Ityfuzz
- Examples
- Video examples
- Ityfuzz in the
test/Fuzz
folder
We can run ityfuzz in two main ways:
-
Running ityfuzz on Invariant Tests Use ityfuzz to run on top of the invariant tests you have already written. This method leverages the existing invariant checks in your tests.
To use inline assertions with ityfuzz, follow these steps:
Step 1: Incorporate the solidity_utils library into your contract (LiquidityMine.sol):
import "lib/solidity_utils/lib.sol";
Step 2: Add the cheat code bug() at the points in your code where you want to verify an invariant.
- test/Fuzz/ItyfuzzInvariant.t.sol
ityfuzz evm -m ItyfuzzInvariant -- forge build
ityfuzz evm -m test/Fuzz/ItyfuzzInvariant.t.sol:ItyfuzzInvariant -- forge test --mc ItyfuzzInvariant --mt <test>
- CheatCode
- Documentation
- Formal Verification In Practice: Halmos, Hevm, Certora, and Ityfuzz
- Examples
- Kontrol in the test/FormalVerification folder
- test/FormalVerification/KontrolFV.t.sol
To use Kontrol effectively, follow these steps:
Step 1: Recompile Your Project
In most cases, you need to recompile your project:
forge build --force
Step 2: Compile Kontrol
Next, compile Kontrol:
kontrol build
Step 3: Execute Tests
You can execute your tests in several ways. Here is a clear method using the command:
kontrol prove --match-test KontrolFVLiquidityKontrol.<test> --max-depth 10000 --no-break-on-calls --max-frontier-parallel 2 --verbose
Step 4: Investigate Test Failures
If you want to delve deeper into a test to understand why it failed, you can use the following commands for a detailed breakdown of the interaction:
kontrol view-kcfg 'KontrolFVLiquidityKontrol.testFuzz_Deposit(uint256,address)' --version <specify version>
or
kontrol view-kcfg KontrolFVLiquidityKontrol.testFuzz_Deposit
or
kontrol show KontrolFVLiquidityKontrol.testFuzz_Deposit