code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

`Equity.restructureCapTable` only restructures one address at a time #972

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Equity.restructureCapTable allows qualified FPS holders to restructure the system, that is: burning shares of other holders that did not participate in putting equity above water.

File: Equity.sol
309: function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
310:         require(zchf.equity() < MINIMUM_EQUITY);
311:         checkQualified(msg.sender, helpers);
312:         for (uint256 i = 0; i<addressesToWipe.length; i++){
313:             address current = addressesToWipe[0];//@audit should be addressesToWipe[i]
314:             _burn(current, balanceOf(current));
315:         }
316:     }

The issue is that the loop sets on each iteration the address to remove as addressesToWipe[0], instead of addressesToWipe[i].

Impact

restructureCapTable() does not perform its function: removing atomically all the "passive" shareholders. This not only means increased gas cost - as the qualified holder will need to call Equity.restructureCapTable() N times to remove N holders, but also makes the whole procees cumbersome if the function needs to be called a high number of times

Proof Of Concept

Run the test test_Audit_Equity_RestructureIncorrect from the following gist: https://gist.github.com/joestakey/fbc954de53b7e018f5121212488e847c

Only the first address gets wiped out.

Tools Used

Manual Analysis, Foundry

Mitigation

File: Equity.sol
309: function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
310:         require(zchf.equity() < MINIMUM_EQUITY);
311:         checkQualified(msg.sender, helpers);
312:         for (uint256 i = 0; i<addressesToWipe.length; i++){
+313:             address current = addressesToWipe[i];
-313:             address current = addressesToWipe[0];
314:             _burn(current, balanceOf(current));
315:         }
316:     }
c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #941

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory