Malicious owner could frontrun a stake transaction with a `setWarmUpPeriod`, permalocking the funds

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L226-L229 https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Staking.sol#L406-L450

Vulnerability details

Here's the interesting part in the stake method:

File: Staking.sol
406:     function stake(uint256 _amount, address _recipient) public {
...
437:         } else {
438:             // create a claim and mint tokens so a user can claim them once warm up has passed
439:             warmUpInfo[_recipient] = Claim({
440:                 amount: info.amount + _amount,
441:                 credits: info.credits +
442:                     IYieldy(YIELDY_TOKEN).creditsForTokenBalance(_amount),
443:                 expiry: epoch.number + warmUpPeriod
444:             });
445: 
446:             IYieldy(YIELDY_TOKEN).mint(address(this), _amount);
447:         }
...
450:     }

A malicious owner could frontrun a stake transaction with a setWarmUpPeriod, permalocking the funds:

File: Staking.sol
226:     function setWarmUpPeriod(uint256 _vestingPeriod) external onlyOwner {
227:         warmUpPeriod = _vestingPeriod;
228:         emit LogSetWarmUpPeriod(block.number, _vestingPeriod);
229:     }

Mitigation

Consider providing a sensible upper limit to warmUpPeriod of, say, a month. This will give raise trust on the fact that a user's funds won't be locked forever

[toshiSat]

sponsor confirmed: i think an upper limit would be worth doing

[0xean]

hmmm, yes, but a malicious owner is a pretty big assumption and that same malicious owner could simply upgrade these contracts to whatever they wanted.

[JasoonS]

Yeah, when there are upgradeable contracts in the mix lots of these kinds of things become irrelevant.

But this definitely is an issue with the system outside of upgradeability.

Can make it a LOW.