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

Single-Step Ownership Transfer Risk with Ownable

drmanhattan852

[L-6] Single-Step Ownership Transfer Risk with Ownable

Description

The contract uses OpenZeppelin's Ownable instead of Ownable2Step, which increases the risk of accidental ownership transfers to incorrect addresses. With Ownable, ownership transfers happen immediately without requiring confirmation from the new owner.

contract BriVault is ERC4626, Ownable {
// ...
constructor (...) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
// ...
}
// ...
}

Risk

Likelihood:

  • Ownership transfers require only one transaction from the owner

  • Human error in address input occurs regularly in blockchain operations

Impact:

  • Permanent loss of administrative control if ownership is transferred to an incorrect address

  • Critical protocol functions like setWinner would become inaccessible, potentially locking all user funds

Proof of Concept

Owner initiates transfer to address 0x8e53A2...1b7F but mistypes a character, sending ownership to 0x8e53A2...1b7E. Since no acceptance is required, ownership is immediately and permanently lost to the incorrect address.

// Deploy the contract with owner = deployer
BriVault vault = new BriVault(...);
​
// Owner attempts to transfer ownership but enters wrong address
address wrongAddress = 0x8e53A2...1b7E;
vault.transferOwnership(wrongAddress);
​
// Ownership is immediately transferred with no confirmation
// Now attempting to call owner-only functions fails
vault.setWinner(5); // Reverts because msg.sender != owner
​
// No recovery mechanism exists - contract is permanently bricked
// All user funds potentially locked forever

Recommended Mitigation

Replace Ownable with OpenZeppelin's Ownable2Step implementation, which requires the new owner to claim ownership before transfer completion, preventing accidental transfers to incorrect addresses.

// Import Ownable2Step
import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
​
// Change inheritance
contract BriVault is ERC4626, Ownable2Step {
constructor (...) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable2Step() {
_transferOwnership(msg.sender);
// rest of constructor
}
// With this implementation:
// 1. Owner calls transferOwnership(newOwner)
// 2. newOwner must call acceptOwnership()
// 3. If wrong address, original owner retains control
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!