First Flight #19: Mondrian Wallet v2
First Flight #19
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Centralization Risk

mishraji874

Description: The contract uses a single-owner model for the upgrades, creating a central point of failure. If the owner's key is compromised, an attacker could upgrade the contract to a malicious implementation.
The contract uses a UUPS upgradeable pattern, but the _authorizeUpgrade function is empty, allowing the owner to upgrade the contract without any restrictions.

contract MondrianWallet2 is IAccount, Initializable, OwnableUpgradeable, UUPSUpgradeable {

and

function _authorizeUpgrade(address newImplementation) internal override {}

Impact:

  1. An attacker gains access to the owner's private key.

  2. The attacker deploys a new malicious implementation contract.

  3. The attacker calls the upgrade function to replace the current implementation with the malicious one.

  4. All future interactions with the wallet now use the malicious code, potentially allowing the attacker to steal funds or perform other malicious actions.

  5. The owner could maliciously upgrade the contract, potentially draining user funds or introducing backdoors.

Recommended Mitigation:

Implement a timelock mechanism for upgrade:

uint256 public constant UPGRADE_TIMELOCK = 2 days;
address public pendingImplementation;
uint256 public upgradeProposalTime;
​
function proposeUpgrade(address newImplementation) external onlyOwner {
pendingImplementation = newImplementation;
upgradeProposalTime = block.timestamp;
emit UpgradeProposed(newImplementation);
}
​
function _authorizeUpgrade(address newImplementation) internal override {
require(newImplementation == pendingImplementation, "Invalid implementation");
require(block.timestamp >= upgradeProposalTime + UPGRADE_TIMELOCK, "Timelock not expired");
pendingImplementation = address(0);
upgradeProposalTime = 0;
emit UpgradeAuthorized(newImplementation);
}
​
event UpgradeProposed(address indexed newImplementation);
event UpgradeAuthorized(address indexed newImplementation);

This solution will add a timelock period between proposing and executing an upgrade, giving users time to react if a malicious upgrade is proposed. It also improves transparency by emitting events for upgrade-related actions.

Updates

Lead Judging Commences

bube Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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