Fjord Token Staking

Vulnerability Details

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L347

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordPoints.sol#L163

Impact

Some critical functions have onlyOwner modifier, which allows onlyOwner role to transfer ownership to another address. It's possible that the current owner mistakenly transfers ownership to the wrong address, resulting in a loss of the onlyOwner role. In the staking contract, the current ownership transfer process involves the current owner calling setOwner(). This function checks the new owner is not the zero address and proceeds to write the new owner's address into the owner's state variable. If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, breaking all functions with the onlyOwner() modifier. Lack of two-step procedure for critical operations leaves them error-prone if the address is incorrect, the new address will take on the functionality of the new role immediately

Tools Used

Manual Review

Recommendations

Consider implementing a two step process using [openzeppeling ownable contract](https://docs.openzeppelin.com/contracts/4.x/api/access#Ownable2Step) where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.