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

5 stars 4 forks source link

When the reserve has zero shares everybody has the power to deny everybody's position #952

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

When the Frankencoin and Reserve contract are first deployed and no reserve shares have been minted, a malicious user can thwart and call deny on any Position, wasting large amounts of their gas and preventing them from minting Frankencoin.

Proof of Concept

    //@audit inside Position.sol
    function deny(address[] calldata helpers, string calldata message) public {

        if (block.timestamp >= start) revert TooLate();

        //@audit here it is checked if the msg.sender is qualified to deny this position
        IReserve(zchf.reserve()).checkQualified(msg.sender, helpers);
        cooldown = expiration; // since expiration is immutable, we put it under cooldown until the end
        emit PositionDenied(msg.sender, message);
    }

    //@audit inside Equity.sol
    function checkQualified(address sender, address[] calldata helpers) public override view {
        uint256 _votes = votes(sender, helpers);
        //@audit here if _votes and totalVotes() are both zero then this conditional wont revert and the denial request will go through
        if (_votes * 10000 < QUORUM * totalVotes()) revert NotQualified();
    }

Tools Used

Manual Review

Recommended Mitigation Steps

In addition to a percentage of the total pool, require a minimum amount of reserve shares before an individual is qualified to vote

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as primary issue

0xA5DF commented 1 year ago

Severity seems med, since no funds are lost

luziusmeisser commented 1 year ago

The operating assumption is that whoever deploys the smart contract will mint the first shares immediately afterwards.

And even if that did not happen, whoever creates the first position could simply create a few pool shares before doing so.

--> Not a relevant issue.

c4-sponsor commented 1 year ago

luziusmeisser marked the issue as sponsor disputed

c4-judge commented 1 year ago

hansfriese changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

hansfriese marked the issue as grade-c