BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Unauthorized Ownership Transfer Allows Takeover of Contract

wyd_derick

Root + Impact

Description

  • The constructor mixes two different ownership initialization styles Ownable(msg.sender) and transferOwnership(msg.sender) even though OpenZeppelin’s Ownable already sets the deployer as the owner.


  • Because of this redundancy, depending on inheritance order and compiler behavior, ownership can be set to the zero address or assigned to an unintended account, allowing a potential unauthorized contract takeover or permanent loss of control.

constructor(
IERC20 _asset,
uint256 _participationFeeBsp,
uint256 _eventStartDate,
address _participationFeeAddress,
uint256 _minimumAmount,
uint256 _eventEndDate
)
ERC4626(_asset)
ERC20("BriTechLabs", "BTT")
@> Ownable(msg.sender) // redundant — OpenZeppelin already sets the deployer as owner
{
@> transferOwnership(msg.sender); // second redundant ownership assignment
}
​

Risk

Likelihood:

  • Occurs during deployment or upgrade if wrong constructor version is used.

  • Can leave ownership uninitialized (owner = address(0)).

Impact:

  • If ownership is address(0), onlyOwner functions become permanently locked (denial of control).

  • If misassigned, an attacker or external deployer could take over all privileged actions (e.g., setting fees, event winners, rescuing funds).

Proof of Concept

contract BriVault is ERC4626, Ownable {
constructor(IERC20 _asset)
ERC4626(_asset)
ERC20("BriTechLabs", "BTT")
Ownable(msg.sender) // may mis-set or duplicate ownership logic
{}
}

Explanation:
If the OpenZeppelin Ownable base constructor already sets the deployer as the owner,
adding Ownable(msg.sender) again may pass a zero or unintended address depending on inheritance order.
This can result in owner() returning the wrong value — enabling unauthorized users to execute privileged actions or permanently locking out legitimate control.

Recommended Mitigation

Summary: Removing both Ownable(msg.sender) and transferOwnership(msg.sender) prevents ownership confusion, aligns with OpenZeppelin standards, and ensures the deployer remains the legitimate contract owner.

- constructor(
- IERC20 _asset,
- uint256 _participationFeeBsp,
- uint256 _eventStartDate,
- address _participationFeeAddress,
- uint256 _minimumAmount,
- uint256 _eventEndDate
- ) ERC4626(_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
- transferOwnership(msg.sender);
- }
​
+ constructor(
+ IERC20 _asset,
+ uint256 _participationFeeBsp,
+ uint256 _eventStartDate,
+ address _participationFeeAddress,
+ uint256 _minimumAmount,
+ uint256 _eventEndDate
+ ) ERC4626(_asset) ERC20("BriTechLabs", "BTT") {
+ // OpenZeppelin automatically sets deployer as owner
+ require(_participationFeeAddress != address(0), "Invalid fee address");
+ }
​
Updates

Appeal created

bube Lead Judge 20 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!