Frankencoin - ToonVH's results

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L313

Vulnerability details

Impact

In restructureCapTable() in Equity.sol current is always set to addressesToWipe[0] instead of addressesToWipe[i]. As a result only the first address of addressesToWipe will have its FPS burned, all the others will remain intact.

Tools Used

Manual review

Recommended Mitigation Steps

Change addressesToWipe[0] to addressesToWipe[i]

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L290-L296 https://github.com/code-423n4/2023-04-frankencoin/blob/main/contracts/Equity.sol#L266-L269

Vulnerability details

Impact

An attacker who holds FPS can backrun a user's transaction that invests in FPS to increase the price at which he redeems his FPS. This is because the user's tx will cause the equity to rise, and thus the FPS redeem price is higher.

A large constraint on this attack is that the attacker must hold his FPS for 90 days to reach the MIN_HOLDING_DURATION before being allowed to redeem.

This works for every transaction that increases the equity: investments into FPS, opening positions, suggesting minters...

Similarly this can be done in reverse for transactions that decrease equity. In comparison to the original attack, this causes a direct loss for the user (since he receives less ZCHF for his FPS)

1. Attacker redeems his FPS
2. User sends transaction that decreases equity (redemption of FPS, sell collateral at loss, ..)
3. Attacker deposits FPS at cheaper price

Proof of Concept

Place this in GeneralTest.t.sol

function test_sandwich() public {
    // init the system 
    Equity equity = Equity(address(zchf.reserve()));
    initPosition();
    alice.obtainFrankencoins(swap, 1500 ether);
    alice.invest(1500 ether);


    // attacker invests in FPS
    bob.obtainFrankencoins(swap, 2000 ether);
    bob.invest(2000 ether);
    uint256 fps = equity.balanceOf(address(bob));
    
    // 100 days pass
    vm.roll(block.number + (100*7200)); // move blocknr 100 days forward

    // user performs tx that increases equity
    alice.obtainFrankencoins(swap, 2000 ether);
    alice.invest(2000 ether);
    //initPosition(); //Instead of an invest() tx, initPosition txs are also vulnerable

    // attacker redeems his FPS at higher price.
    vm.prank(address(bob));
    equity.redeem(address(bob), fps);
    console.log(zchf.balanceOf(address(bob))); // attacker gets back 2611 ZCHF 
}

Tools Used

Manual review, Foundry for PoC

Recommended Mitigation Steps

• Not a complete solution, but it might be useful to add an 'expected_amount' argument to the onTokenTransfer() and redeem() functions. This would prevent the user to lose funds for the reverse attack.