Back to reassor's contests

Yieldy contest - reassor's results

A protocol for gaining single side yields on various tokens.

General Information

Platform: Code4rena

Start Date: 21/06/2022

Pot Size: $50,000 USDC

Total HM: 31

Participants: 99

Period: 5 days

Judges: moose-code, JasoonS, denhampreen

Total Solo HM: 17

Id: 139

League: ETH

Yieldy

Findings Distribution

Researcher Performance

Rank: 42/99

Findings: 2

Award: $119.21

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryYieldy

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1337, 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 0xc0ffEE, 0xf15ers, 0xmint, Bnke0x0, BowTiedWardens, Chom, ElKu, FudgyDRS, Funen, GalloDaSballo, GimelSec, JC, Kaiziron, Lambda, Limbooo, Metatron, MiloTruck, Noah3o6, Picodes, PumpkingWok, PwnedNoMore, Sm4rty, StErMi, TomJ, TrungOre, UnusualTurtle, Waze, _Adam, aga7hokakological, ak1, antonttc, berndartmueller, cccz, cryptphi, csanuragjain, defsec, delfin454000, dipp, elprofesor, exd0tpy, fatherOfBlocks, hake, hansfriese, hubble, joestakey, kenta, ladboy233, mics, oyc_109, pashov, pedr02b2, reassor, robee, samruna, scaraven, shung, sikorico, simon135, sseefried, tchkvsky, unforgiven, zzzitron

Awards

84.4957 USDC - $84.50

Labels

bug
QA (Quality Assurance)

External Links

Link to GitHub issue

1. Migration incorrect interface

Impact

Contract Migration implements function moveFundsToUpgradedContract that is expected to move funds from OLD_CONTRACT to NEW_CONTRACT. The issue is that for withdrawing funds it uses instantUnstake that does not exist in Staking contract. Contract Staking implements instantUnstakeReserve and instantUnstakeCurve, while instantUnstake is being implemented by LiquidityReserve.

Proof of Concept

Migration.sol:

Tools Used

Manual Review / VSCode

It is recommended to use ILiquidityReserve interface for OLD_CONTRACT and make sure that OLD_CONTRACT is a LiquidityReserve contract.

2. Missing threshold validation

Risk

Low

Impact

Contract Staking implements multiple functions for setting staking parameters. The issue is that these functions are missing basic threshold checks of received arguments which makes it risky that, either by accident or intentionally, the parameters will be set to values that will completely break Staking contract logic.

Proof of Concept

Staking.sol:

Tools Used

Manual Review / VSCode

It is recommended to add threshold checks for listed parameters.

3. Missing approve(0) first

Risk

Low

Impact

Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

IERC20(token).approve(address(operator), 0);
IERC20(token).approve(address(operator), amount);

Proof of Concept

Staking.sol:

LiquidityReserver.sol:

Migration.sol:

Tools Used

Manual Review / VSCode

Approve with a zero amount first before setting the actual amount.

4. Missing events

Risk

Low

Impact

Multiple contracts are not implementing events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.

Proof of Concept

Staking.sol:

LiquidityReserve.sol:

Migration.sol:

BatchRequests.sol:

Tools Used

Manual Review / VSCode

It is recommended to add missing events to listed functions.

5. Missing zero address checks

Risk

Low

Impact

Multiple contracts do not check for zero addresses which might lead to loss of funds, failed transactions and can break the protocol functionality.

Proof of Concept

Staking.sol:

LiquidityReserve.sol:

BatchRequests.sol:

Tools Used

Manual Review / VSCode

It is recommended to add zero address checks for listed parameters.

6. Critical address change

Risk

Low

Impact

Changing critical addresses such as ownership should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one. This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible.

Proof of Concept

Staking.sol:

LiquidityReserve.sol:

BatchRequests.sol:

Tools Used

Manual Review / VSCode

It is recommended to implement two-step process for changing ownership.

7. Use max values

Risk

Non-Critical

Impact

Contract YieldStorage.sol uses bit negation to retrieve MAX_UINT256 and MAX_SUPPLY (max uint128), these values can be easily retrieved from type object.

(..)
    uint256 internal constant MAX_UINT256 = ~uint256(0);

    uint256 internal constant MAX_SUPPLY = ~uint128(0); // (2^128) - 1
(..)

Proof of Concept

YieldyStorage.sol:

Tools Used

Manual Review / VSCode

It is recommended to use type(uint256).max and type(uint128).max respectively.

8. Use scientific notation

Risk

Non-Critical

Impact

    uint256 public constant BASIS_POINTS = 10000; // 100% in basis points

Proof of Concept

StakingStorage.sol:

LiquidityReserveStorage.sol:

Tools Used

Manual Review / VSCode

It is recommended to use scientific notation such as 1e4.

9. Missing/incomplete natspec comments

Risk

Non-Critical

Impact

Contracts are missing natspec comments which makes code more difficult to read and prone to errors.

Proof of Concept

Staking.sol:

Yieldy.sol:

Migration.sol:

BatchRequests.sol:

Tools Used

Manual Review / VSCode

It is recommended to add missing natspec comments.

Findings Information

🌟 Selected for report: BowTiedWardens

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, 8olidity, ACai, Bnke0x0, Chom, ElKu, Fabble, Fitraldys, FudgyDRS, Funen, GalloDaSballo, GimelSec, IllIllI, JC, Kaiziron, Lambda, Limbooo, MiloTruck, Noah3o6, Nyamcil, Picodes, PwnedNoMore, Randyyy, RedOneN, Sm4rty, StErMi, TomJ, Tomio, TrungOre, UnusualTurtle, Waze, _Adam, aga7hokakological, ajtra, antonttc, asutorufos, bardamu, c3phas, defsec, delfin454000, exd0tpy, fatherOfBlocks, hansfriese, ignacio, joestakey, kenta, ladboy233, m_Rassska, mics, minhquanym, oyc_109, pashov, reassor, robee, s3cunda, sach1r0, saian, sashik_eth, scaraven, sikorico, simon135, slywaters

Awards

34.7125 USDC - $34.71

Labels

bug
G (Gas Optimization)

External Links

Link to GitHub issue

1. Use scientific notation

Impact

Contract LiquidityReserveStorage is using math exponent calculation to express big numbers. This consumes additional gas and it is better to use scientific notation.

Proof of Concept

LiquidityReserveStorage.sol:9:    uint256 public constant MINIMUM_LIQUIDITY = 10**3; // lock minimum stakingTokens for initial liquidity

Tools Used

Manual Review / VSCode

It is recommended to use scientific notation, for example: 1e3.

2. Pack integer values

Impact

Packing integer variables into storage slots saves gas.

struct Epoch {
    uint256 duration; // length of the epoch (in seconds)
    uint256 number; // epoch number (starting 1)
    uint256 timestamp; // epoch start time
    uint256 endTime; // time that current epoch ends on
    uint256 distribute; // amount of rewards to distribute this epoch
}

In order to save gas following variables can be packed:

uint256 duration -> uint32 duration (alternatively uint64)
uint256 number -> uint32 number (alternatively uint64)
uint256 timestamp -> uint32 timestamp (alternatively uint64)
uint256 endTime -> uint32 endTime (alternatively uint64)

This will allow pack struct's 5 slots into 2 slots.

Proof of Concept

structs/Epoch.sol:

Tools Used

Manual Review / VSCode

It is recommended to pack listed integer variables.

3. Use custom errors instead of revert strings to save gas

Impact

Usage of custom errors reduces the gas cost.

Proof of Concept

Contracts that should be using custom errors:

  • LiquidityReserve.sol
  • Migration.sol
  • Staking.sol
  • Yieldy.sol

Tools Used

Manual Review / VSCode

It is recommended to add custom errors to listed contracts.

4. ++i/--i costs less gas compared to i++, i += 1, i-- or i -= 1

Impact

++i or --i costs less gas compared to i++, i += 1, i-- or i -= 1 for unsigned integer as pre-increment/pre-decrement is cheaper (about 5 gas per iteration).

Proof of Concept

Staking.sol:708:            epoch.number++;

Tools Used

Manual Review / VSCode

It is recommended to use ++i or --i instead of i++, i += 1, i-- or i -= 1 to increment value of an unsigned integer variable.

5. Obsolete overflow/underflow check

Impact

Starting from solidity 0.8.0 there is built-in check for overflows/underflows. This mechanism automatically checks if the variable overflows or underflows and throws an error. Multiple contracts use increments that cannot overflow but consume additional gas for checks.

Proof of Concept

Staking.sol:708:            epoch.number++;

Tools Used

Manual Review / VSCode

It is recommended to wrap incrementing with unchecked block, for example: unchecked { ++i } or unchecked { --i }

6. No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.

Proof of Concept

Staking.sol:636:            int128 from = 0;
Staking.sol:637:            int128 to = 0;

Tools Used

Manual Review / VSCode

It is recommended to remove explicit initializations with default values.

7. Use != 0 instead of > 0 for unsigned integer comparison

Impact

When dealing with unsigned integer types, comparisons with != 0 are cheaper than with > 0.

Proof of Concept

LiquidityReserve.sol:223:            if (amount > 0) IStaking(stakingContract).unstake(amount, false);
Staking.sol:118:        require(_recipient.amount > 0, "Must enter valid amount");
Staking.sol:305:            requestedWithdrawals.amount > 0 &&
Staking.sol:326:        if (amountToRequest > 0) tokePoolContract.requestWithdrawal(_amount);
Staking.sol:363:            requestWithdrawalAmount > 0;
Staking.sol:392:            if (requestWithdrawalAmount > 0) {
Staking.sol:410:        require(_amount > 0, "Must have valid amount");
Staking.sol:415:        if (yieldyTotalSupply > 0) {
Staking.sol:470:            if (info.credits > 0) {
Staking.sol:533:        if (warmUpBalance > 0) {
Staking.sol:572:        require(_amount > 0, "Invalid amount");
Staking.sol:604:        require(_amount > 0, "Invalid amount");
Yieldy.sol:83:        require(_totalSupply > 0, "Can't rebase if not circulating");
Yieldy.sol:96:            require(rebasingCreditsPerToken > 0, "Invalid change in supply");

Tools Used

Manual Review / VSCode

It is recommended to use != 0 instead of > 0.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter