Yieldy contest - kenta's results

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Migration.sol#L48 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L471

Vulnerability details

Impact

transfer and transferFrom in Yieldy.sol return the bool if the execution is successful. You can check these return value in the following lines.

Proof of Concept

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Migration.sol#L48 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L471

Tools Used

code review

Recommended Mitigation Steps

require( IYieldy(OLD_YIELDY_TOKEN).transferFrom(msg.sender, address(this), userWalletBalance), "transfer is not completed" );

require( IYieldy(YIELDY_TOKEN).transfer(_recipient, IYieldy(YIELDY_TOKEN).tokenBalanceForCredits(info.credits)), "transfer is not completed" );

2022-06-yieldy

1 typo ?

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/BatchRequests.sol#L86

@notice remove the address from contracts array

2 the missing validation

The pram _address must be checked if the _address is empty or not.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/BatchRequests.sol#L81-L83

require(_address != address(0), “EMPTY ADDRESS”);

2 use mixedCase instead of all capital letters with underscores separating words.

According to the solidity doc, Constants should be named with all capital letters with underscores separating words. However, it is not for the immutable.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Migration.sol#L14-L16

Foe example,

address public immutable oldContract;

2022-06-yieldy gas optimization

1 use the params instead of the state variable.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L43-L46

WAD = 10_decimal; rebasingCreditsPerToken = 10_decimal; _setIndex(10**_decimal);

2 use the cache which is defined before in rebase.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L83

require(currentTotalSupply > 0, "Can't rebase if not circulating");

3 use a cache for _totalSupply in _storeRebase

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L122 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L129

uint256 totalSupply = _totalSupply; And this cache for the above lines.

4 use a cache for getIndex() in _storeRebase.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L124 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L130

uint256 index = getIndex(); And this cache for above lines.

5 use a cache for _totalSupply + _amount to avoid the sload in mint.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L255 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L257

uint256 updatedTotalSupply = _totalSupply + _amount; And this cache for above lines.

6 use cache for creditBalances[_address] in _burn.

creditBalances[_address] is defined as a cache with line 285. Use it for the following line.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L288

7 use unchecked for the following calculation in _burn.

Before the following line, the underflow is already checked with line 286. Use unchecked for the calculation.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L288

unchecked { creditBalances[_address] = creditBalances[_address] - creditAmount; }

8 use a cache for _allowances[_from][msg.sender] in transferFrom.

_allowances[_from][msg.sender] is used twice in transferFrom. Use cache for them.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L210 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L212

For example, uint256 allowance = _allowances[_from][msg.sender]; Use it for the above lines.

9 use a cache for creditBalances[msg.sender] in transfer.

creditBalances[msg.sender] is used twice in transfer. Use a cache and use it for the following lines.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L190 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Yieldy.sol#L192

For example, uint256 currentCreditBalance = creditBalances[msg.sender];

10 don’t use the cache for ITokeReward(TOKE_REWARD) in claimFromTokemak.

tokeRewardContract is used only one time in laimFromTokemak. Do not use the cache for it.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L120

ITokeReward(TOKE_REWARD).tokeRewardContract.claim(_recipient, _v, _r, _s);

11 use a cache for IERC20Upgradeable(TOKE_TOKEN) in transferToke.

IERC20Upgradeable(TOKE_TOKEN) is used twice in transferToke. Use a cache to save gas costs.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L144 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L147

IERC20Upgradeable tokeToken = IERC20Upgradeable(TOKE_TOKEN);

Use it for the above lines.

12 do not use the caches tokeManager and tokePoolContract.

Both are used only one time each other. Don’t use these caches.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L279 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L280 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L299 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L334 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L343 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L372

RequestedWithdrawalInfo memory requestedWithdrawals = ITokePool(TOKE_POOL).requestedWithdrawals(address(this)); uint256 currentCycleIndex = ITokeManager(TOKE_MANAGER).getCurrentCycleIndex();

13 use storage instead of memory in the following line.

You use this cache only two times in this function, so storage is cheaper than memory.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L466

use unchecked for balance - staked in rebase.

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/Staking.sol#L716

unchecked { epoch.distribute = balance - staked;
}