`restructure` can be front run

[code423n4]

Lines of code

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

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];
314:             _burn(current, balanceOf(current));
315:         }
316:     }

The issue is that the process cannot be done atomically: the "brave souls" have to call restructureCapTable() first, then donate zchf.

Impact

A malicious shareholder (holding more than 3% of the totalShares) can front run the call to restructureCapTable() by legitimate users ready to give zchf, calling restructureCapTable() to burn the shares of the "brave souls" that made the initial call.

Note this attack is even more likely that at this stage, the malicious shareholder have nothing to lose: their shares were worth nothing (because of equity being underwater) and they were about to be "wiped out" by the legitimate users call to restructureCapTable().

Tools Used

Manual Analysis

Mitigation

To avoid this griefing attack, a solution is to make the bootstrapping process atomic: the callers of restructureCapTable() will transfer zchf in the call

-function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
+function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe, uint256 amount_) public {
    require(zchf.equity() < MINIMUM_EQUITY);
    checkQualified(msg.sender, helpers);
    for (uint256 i = 0; i<addressesToWipe.length; i++){
        address current = addressesToWipe[i];
        _burn(current, balanceOf(current));
    }
+   zchf.transferFrom(msg.sender, address(this), amount_);
+   require(zchf.equity() >= MINIMUM_EQUITY, "equity still at loss");
}

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #571

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #132

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-b

[joestakey]

Hi @hansfriese , can you explain why this issue (and other duplicates) was downgraded? The griefing attack is very likely to happen as it is easy to perform, breaking the entire restructuring functionality , which falls under the function of the protocol or its availability could be impacted Medium category of the docs. On top of that the mitigation suggested would get rid of the issue entirely.