Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Lack of a two-step Transfer of Ownership

JJS

Summary

In a few instances in the codebase, a transfer of ownership occurs, however there is a lack of a two-step transfer of ownership, and no use of the Ownable contract from OpenZeppelin.
Here are the instances:

  • FjordPoints.sol

function setOwner(address _newOwner) external onlyOwner {
if (_newOwner == address(0)) revert InvalidAddress();
owner = _newOwner;
}
​
function setStakingContract(address _staking) external onlyOwner {
if (_staking == address(0)) {
revert InvalidAddress();
}
​
staking = _staking;
}

  • FjordAuctionFactory.sol:

function setOwner(address _newOwner) external onlyOwner {
if (_newOwner == address(0)) revert InvalidAddress();
owner = _newOwner;
}

  • FjordStaking.sol:

function setOwner(address _newOwner) external onlyOwner {
if (_newOwner == address(0)) revert InvalidZeroAddress();
owner = _newOwner;
}
​
function setRewardAdmin(address _rewardAdmin) external onlyOwner {
if (_rewardAdmin == address(0)) revert InvalidZeroAddress();
rewardAdmin = _rewardAdmin;
}

Impact

One step transfer includes passing the new address to a setter function and then the transfer is finished. If any issues occur, such as passing an incorrect address to the setters, then important capabilities are lost:

  • Resetting the owner and other roles.

  • Set points per epoch (FjordPoints.sol).

  • Create an auction (FjordAuctionFactory.sol)

Exploit Scenario

Alice, the owner, types in Bob's address, the new owner, to transfer ownership. However, Alice made a typo, and therefore losing ownership of the system.

Recommendations

Implement a two-step transfer of ownership where the new owner also has to confirm to transfer the ownership, also using the OpenZeppelin Ownable.

import "@openzeppelin/contracts/access/Ownable.sol";
address private _newOwner;
​
event OwnershipTransferInitiated(address indexed newOwner);
​
function initiateOwnershipTransfer(address newOwner) public onlyOwner {
require(newOwner != address(0), "New owner is the zero address");
_newOwner = newOwner;
emit OwnershipTransferInitiated(newOwner);
}
​
function acceptOwnership() public {
// Make sure that the addresses match to complete the transfer of ownership
require(msg.sender == _newOwner, "Only proposed owner can accept it");
_transferOwnership(_newOwner);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!