Frankencoin - 0xWeiss's results

Lines of code

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

Vulnerability details

Impact

While calling restructureCapTable is used according to the docs to:

 * Frankencoin system except for a group of small FPS holders who still believes in it and is willing to 
  provide
 * 2'000'000 ZCHF to save it. These brave souls are essentially donating 1'000'000 to the minter reserve and 
 it
 * would be wrong to force them to share the other million with the passive FPS holders. Instead, they will 
 get
 * the possibility to bootstrap the system again owning 100% of all FPS shares.

therefore the address[] calldata addressesToWipe param is given as an array of addresses to burn all the tokens from to make the new "investors" hold 100% of the shares:

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

the problem here is that a hardcoded line:

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

means that you can only burn the balanceOf 1 address instead of all the addresses that you want because it has a hardcoded [0] meaning the first address from the array addressesToWipe[0];

All the transactions to that function will fail because only shares from 1 address can be burned, meaning the new investors that saved the system will not be able to hold 100% of the shares because only 1 address at a time can have their shares burned. It is a medium because if the investor wants to burn 200 addresses, he will have to do that 1 by 1 in 200 transactions, costing an average of 5$ x transaction = 1000$ of loss funds for the investor, so there is an economic damage

Proof of Concept

Created a small contract with the exact same error and balances that you can paste in remix for the judge to test easily. Call first the up() function to set balances and then call restructureCapTable with those addresses that have balances as a param. You will notice it fails.

contract Test{
mapping (address => uint)public bal;

  function up()public{
  bal[msg.sender] = 100;
     bal[0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2] = 100;
     bal[0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db] = 100;
     bal[0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB] = 100;
  }

 function restructureCapTable(address[] calldata addressesToWipe) public {

    for (uint256 i = 0; i<addressesToWipe.length; i++){
        address current = addressesToWipe[0];
        bal[current] = bal[current] -20;
    }
}
 }

Tools Used

Manual

Recommended Mitigation Steps

Should be address current = addressesToWipe[i]; changing the 0 for the i

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/PositionFactory.sol#L44

Vulnerability details

Impact

The createClone function deploys a clone contract using the create, where the address derivation depends only on the PositionFactory nonce.

Re-orgs can happen in all EVM chains. In ethereum, where currently Frankencoin is deployed, it is not "super common" but it still happens, being the last one less than a year ago:

https://decrypt.co/101390/ethereum-beacon-chain-blockchain-reorg

The issue increases the changes of happening because frankencoin is thinking about deploying also in L2's/ rollups, proof:

https://discord.com/channels/810916927919620096/1095308824354758696/1096693817450692658

where re-orgs have been much more active:

https://protos.com/polygon-hit-by-157-block-reorg-despite-hard-fork-to-reduce-reorgs/

being the last one, less than a year ago.

The issue would happen when users rely on the address derivation in advance or try to deploy the position clone with the same address on different EVM chains, any funds sent to the new clone could potentially be withdrawn by anyone else. All in all, it could lead to the theft of user funds.

As you can see in a previous report, the issue should be marked and judged as a medium:

https://code4rena.com/reports/2023-01-rabbithole/#m-01-questfactory-is-suspicious-of-the-reorg-attack

Proof of Concept

Imagine that Alice deploys a position clone, and then sends funds to it. Bob sees that the network block reorg happens and calls clonePosition. Thus, it creates a position clone with an address to which Alice sends funds. Then Alice's transactions are executed and Alice transfers funds to Bob’s position contract.

Tools Used

Manual

Recommended Mitigation Steps

The recommendation is basically the same as:

https://code4rena.com/reports/2023-01-rabbithole/#m-01-questfactory-is-suspicious-of-the-reorg-attack

Deploy the cloned Position via create2 with a specific salt that includes msg.sender and address _existing