Yieldy contest - Limbooo's results

(low) Staking.sol#claim() ignores return value when IYieldy(YIELDY_TOKEN).transfer()

Using transfer and ignoring the return value can lead to an unusual locking on contract fund

Poc

Staking.sol#L465

    function claim(address _recipient) public { 
        Claim memory info = warmUpInfo[_recipient];
        if (_isClaimAvailable(_recipient)) {
            delete warmUpInfo[_recipient];
        ...

After cashing info the data will be deleted, while the success of the transfer in Yieldy contract is not measured at all IYieldy(YIELDY_TOKEN).transfer(...

Tools Used

vscode, hardhat

Recommended Mitigation Steps

Using SafeERC20Upgradeable library to call transfer.

IERC20Upgradeable(YIELDY_TOKEN).safeTransferFrom(
    _user,
    address(this),
    amountLeft
);

---

(low) Avoid leaving a contract uninitialized

In OpenZeppelin Contracts (proxy/utils/Initializable.sol):

[CAUTION]: An uninitialized contract can be taken over by an attacker. This applies to both a proxy and its implementation...

The implementation contract itself should be uninitializable by default to prevent initializing any feature implementation upgrades. Leaving such external function that can be used freely without logical reverting layer.

PoC

This can lead to takeover of 3 implementation contracts: - LiquidityReserve.sol - Staking.sol - Yieldy.sol since implementation contracts not initialized and can be initialized publicly.

Recommendation

As its mentioned in OpenZeppelin Contracts documentation:

To prevent the implementation contract from being used, you should invoke the {_disableInitializers} function in the constructor to automatically lock it when it is deployed:

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

---

(low) Lack of authorization layer

External and public functions can lead to very big lost for the protocol. Function produced to accomplish very deep layer in the core system without knowing the caller integrity is very dangerous in long term.

PoC

BatchRequests.sol #L14

    function sendWithdrawalRequests() external {

Stacking.sol#L111#L384#L406#L465#L485

    function claimFromTokemak(
        Recipient calldata _recipient,
        uint8 _v,
        bytes32 _r,
        bytes32 _s
    ) external {...
    function sendWithdrawalRequests() public {...
    function stake(uint256 _amount, address _recipient) public {...
    function claim(address _recipient) public {...
    function claimWithdraw(address _recipient) public {...

LiquidityReserve.sol#L214

    function unstakeAllRewardTokens() public {...

Tools Used

vscode

Recommended Mitigation Steps

Use a security layer for every intended logic to prevent any logical weakness to be used freely.

---

(non) Solidity version too recent to be trusted

Solidity version used in project is too recent to be trusted

pragma solidity 0.8.9;

PoC

All solidity files

Tools Used

Slither Analysis

Recommended Mitigation Steps

Consider deploying with 0.8.7 if there are no features required from 0.8.9

(inform+gas) Yieldy.sol#transferFrom() will revert on overflow arithmetic operation

Subtraction of uint256 to a negative value in solidity version 0.8.9 will revert.

Poc

transferFrom() is a public function takes the amount and calculate it to creditAmount then suptracte it form creditBalances[_from] where _from is the address to subtract the transfer amount from. However, if the creditAmount is less than creditBalances[_from] will revert upon underflow on arithmetic operation

Yieldy.sol#L217

    creditBalances[_from] = creditBalances[_from] - creditAmount;

Recommendation

• Apply the same creditAmount requirement in Yieldy.sol#transfer() Yieldy.sol#L190

  require(creditAmount <= creditBalances[msg.sender], "Not enough funds");
  

or

• Define an internal function _transfer, since transferFrom() and transfer() implementations share the same logic.

---

(gas) Use Custom Error

Custom Error is a convenient and gas-efficient way to revert errors. the require(_to != address(0), "Invalid address"); uses more line of Yul and can lead to more gas usage.

PoC

LiquidityReserve.sol Migration.sol Stacking.sol Yieldy.sol ...

Recommendation

Define custom error, which can be used inside and outside of contracts (including interfaces and libraries).

error InvalidAddress(address);
...
if (_address == address(0))
    revert InvalidAddress(_address);