Frankencoin - Kek's results

Lines of code

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

Vulnerability details

Context

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

addressesToWipe[0] should be addressesToWipe[i]

Thus, _burn() is only called on the first element of the array.

Impact

This function would have to be called repeatedly with one element in the addressesToWipe array to burn all desired addresses. However, this function is only while the caller is qualified Equity.sol#L311 by having enough delegate votes. checkQualified() checks the votes of each delegate on Equity.sol#L210. Votes() checks the number of votes based on the number of shares (balance) of each delegate @ Equity.sol#L199 > Equity.sol#L180. Thus, the call order could impact this function and prevent it from working if the caller ends up burning shares of his delegates and can no longer pass the checkQualified() check on successive calls; in this case, the caller may never be able to reach 100% ownership of FPS shares.

Additionally, the function allows the caller to specify which addresses to burn shares from without restriction, which means the trusted person to call this function can effectively burn all the shares of the remaining trusted FPS shareholders + those who delegated votes to this individual. This goes against the commented example where only passive FPS holder's shares should be burned - resulting in a possible loss of shares/funds for active FPS shareholders in the event the delegate is malicious or accidentally enters the wrong address into the addressesToWipe parameter of restructureCapTable(). Perhaps a mechanism to prevent burning of active share holders should be considered?

Proof of Concept

bug is clear in code + explination above: https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Equity.sol#L313

Tools Used

VS Code

Recommended Mitigation Steps

• addressesToWipe[0] should be addressesToWipe[i]

• Restrict the function to only burn shares of "passive FPS holders" as opposed to the caller having the ability to burn everybodys shares.