Thunder Loan
AI First Flight #7
Beginner FriendlyFoundryDeFiOracle
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Upgrade Without Timelock or User Notification allows immediate malicious implementation deployment

tvcer

[M-1] Upgrade Without Timelock or User Notification allows immediate malicious implementation deployment

Description

The ThunderLoan and ThunderLoanUpgraded contracts can be upgraded instantly by the owner without any timelock mechanism or advance notification to users. This violates the principle of user protection in upgradeable decentralized protocols.

In the contract, the _authorizeUpgrade function allows the owner to push a new implementation immediately:

Solidity

function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
// No timelock
// No minimum delay
// No notification mechanism
}
​

Because there is zero delay between the proposal of an upgrade and its execution, users have zero time to review the new implementation, withdraw their funds, or adjust their positions before potentially breaking or malicious changes take effect.

Risk

Severity: Medium

  • User Funds at Risk: Users cannot protect their funds before an upgrade is executed.

  • Rug-Pull Potential: A compromised owner key or a malicious owner can instantly upgrade the proxy to a malicious implementation that drains all protocol liquidity in a single transaction.

  • Best Practices Violation: It violates core DeFi best practices for upgradeable protocols.

  • Loss of Trust: The centralization vector erodes user trust and protocol credibility.

Proof of Concept

The following test demonstrates that the protocol owner can deploy a new implementation and upgrade the contract in the exact same transaction, leaving users with 0 seconds to react.

Solidity

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import { Test } from "forge-std/Test.sol";
import { ThunderLoan } from "../../src/protocol/ThunderLoan.sol";
import { ThunderLoanUpgraded } from "../../src/upgradedProtocol/ThunderLoanUpgraded.sol";
​
contract TimelockVulnerabilityTest is Test {
ThunderLoan thunderLoan;
​
function test_InstantUpgradeLeavesNoReactionTimeForUsers() public {
address owner = thunderLoan.owner();
vm.startPrank(owner);
ThunderLoanUpgraded maliciousOrFlawedImpl = new ThunderLoanUpgraded();
// The upgrade happens instantly in the exact same block.
// Users cannot front-run this to save their funds.
thunderLoan.upgradeTo(address(maliciousOrFlawedImpl));
vm.stopPrank();
// Assert the upgrade was successful instantly
assertEq(
address(uint160(uint256(vm.load(address(thunderLoan), 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc)))),
address(maliciousOrFlawedImpl)
);
}
}
​

Recommended Mitigation

Implement a Timelock controller to enforce a mandatory delay between scheduling an upgrade and executing it. This gives users a grace period (e.g., 48 hours) to exit the protocol if they disagree with the proposed changes.

Use OpenZeppelin's TimelockController or implement a custom delay mechanism as shown below:

Solidity

import { TimelockController } from "@openzeppelin/contracts/governance/TimelockController.sol";
​
// Minimum 48-hour delay before upgrade takes effect
uint256 public constant UPGRADE_TIMELOCK = 48 hours;
uint256 public upgradeScheduledAt;
address public pendingImplementation;
​
event UpgradeScheduled(address indexed newImplementation, uint256 scheduledExecutionTime);
​
function scheduleUpgrade(address newImplementation) external onlyOwner {
pendingImplementation = newImplementation;
upgradeScheduledAt = block.timestamp + UPGRADE_TIMELOCK;
emit UpgradeScheduled(newImplementation, upgradeScheduledAt);
}
​
function executeUpgrade() external onlyOwner {
require(block.timestamp >= upgradeScheduledAt, "Timelock not expired");
require(pendingImplementation != address(0), "No upgrade scheduled");
_upgradeTo(pendingImplementation);
pendingImplementation = address(0);
}
​
// Override _authorizeUpgrade to strictly revert if the timelock process isn't followed
function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
require(newImplementation == pendingImplementation, "Must use scheduled implementation");
require(block.timestamp >= upgradeScheduledAt, "Timelock active");
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 hours 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!