HOPR is an open incentivized mixnet which enables privacy-preserving point-to-point data exchange. HOPR is similar to Tor but actually private, decentralized and economically sustainable.
Github username: --
Submission hash (on-chain): 0x92a7480241b5bcbb2fa4d07be5c1f3b42cc96a1bbf51fddd3101dc6de88c828e
Severity: medium
Description:Description\
The ownership pattern implementation is linked to the OwnableUpgradeable contract where a single-step transfer is implemented.
This can lead to problem for all methods marked in onlyOwner throughout the codebase, in which some of have core protocol functionality i.e. NodeManagementModule contract.
Attack Scenario\
Single-step transfer of ownership means, if a wrong address is passed during transferring of ownership it will result in loss of owner based role forever.
SimplifiedModule contract utilizes OwnableUpgradeable contract which is prone to error due to lack effective control.
import { OwnableUpgradeable } from "openzeppelin-contracts-upgradeable/access/OwnableUpgradeable.sol";
Recommendation
It is a best practice to use two-step ownership transfer pattern where ownership transfer gets to a "pending" state and the new owner should claim his new rights, otherwise the old owner will still has control of the contract.
Consider using OpenZeppelin's Ownable2Step contract.
Github username: -- Submission hash (on-chain): 0x92a7480241b5bcbb2fa4d07be5c1f3b42cc96a1bbf51fddd3101dc6de88c828e Severity: medium
Description: Description\
The ownership pattern implementation is linked to the
OwnableUpgradeablecontract where a single-step transfer is implemented.This can lead to problem for all methods marked in onlyOwner throughout the codebase, in which some of have core protocol functionality i.e.
NodeManagementModulecontract.Attack Scenario\
Single-step transfer of ownership means, if a wrong address is passed during transferring of ownership it will result in loss of owner based role forever.
Attachments
https://github.com/hoprnet/hoprnet/blob/master/packages/ethereum/contracts/src/node-stake/permissioned-module/SimplifiedModule.sol#L7
SimplifiedModulecontract utilizesOwnableUpgradeablecontract which is prone to error due to lack effective control.It is a best practice to use two-step ownership transfer pattern where ownership transfer gets to a "pending" state and the new owner should claim his new rights, otherwise the old owner will still has control of the contract.
Consider using OpenZeppelin's Ownable2Step contract.