Yieldy contest - 0xNineDec's results

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Yieldy.sol#L205

Vulnerability details

Impact

The implementation of Yieldy.transferFrom() does not checks that _from currently holds the sent amount _value.

Although the current compiler reverts while having underflows, having a lack of that check in one of the core functions of the main protocol tokens is a considerable flaw. Currently the only factor that is protecting this function from assigning an infinite amount of tokens triggered by an underflowing transfer is the compiler version.

In order to increase the security on how transfers are going to be performed, it is advisable to check that the _from account holds the _value that is desired to be sent.

Lines of code

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

Vulnerability details

Impact

Currently Yieldy._mint() violates the structure of Checks-Effects-Interactions regarding how is computed and controlled the _totalSupply of each Yieldy asset.

Proof of Concept

This is the current implementation:

    Yieldy.sol

    248     function _mint(address _address, uint256 _amount) internal override {
    249         require(_address != address(0), "Mint to the zero address");
    250
    251         uint256 creditAmount = _amount * rebasingCreditsPerToken;
    252         creditBalances[_address] = creditBalances[_address] + creditAmount;
    253         rebasingCredits = rebasingCredits + creditAmount;
    254
    255         _totalSupply = _totalSupply + _amount;
    256
    257         require(_totalSupply < MAX_SUPPLY, "Max supply");
    258         emit Transfer(address(0), _address, _amount);
    259     }

There are many scenarios where violating the Checks-Effects-Interactions design pattern yielded to undesired behavior or unexpected attacks. In order to bring a more secure execution structure, whenever is possible it is advisable to respect the pattern mentioned before.

Recommended Mitigation Steps

The line 257 can be moved to line 250 and changed for the following alternative in order to preserve a more secure design pattern:

require(_totalSupply + _amount < MAX_SUPPLY, "Max supply");