code423n4 commented 3 years ago

Handle

gpersoon

Vulnerability details

Impact

You can keep the DAO voting system hostage by repeatedly adding useless proposals. You would have to monitor the blockchain and as soon as a new proposal can be made, make a new useless proposal. (submit with a high fee and/or via a high priority channel like flashbots, taichi, bloxroute, archer)

A new proposal can only be submitted when mapPID_open[currentProposal] == false (see checkProposal() ) The contract can only reach this state via cancelProposal() or finaliseProposal()&completeProposal() Both cancelProposal() and finaliseProposal() force waiting for quite a while:

cancelProposal ==> 1296000 seconds (15 days)
finaliseProposal ==> coolOffPeriod ==> 259200 seconds (3 days)

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/main/contracts/Dao.sol#L406

function checkProposal() internal { require(mapPID_open[currentProposal] == false, '!open'); // There must not be an existing open proposal proposalCount += 1; // Increase proposal count currentProposal = proposalCount; // Set current proposal to the new count mapPID_open[currentProposal] = true; // Set new proposal as open status mapPID_startTime[currentProposal] = block.timestamp; // Set the start time of the proposal to now }

function cancelProposal() external { require(block.timestamp > (mapPID_startTime[currentProposal] + 1296000), "!days"); // Proposal must not be new mapPID_votes[currentProposal] = 0; // Clear all votes from the proposal mapPID_open[currentProposal] = false; // Set the proposal as not open (closed status) emit CancelProposal(msg.sender, currentProposal); }

// A finalising-stage proposal can be finalised after the cool off period
function finaliseProposal() external {
    require((block.timestamp - mapPID_coolOffTime[currentProposal]) > coolOffPeriod, "!cooloff"); // Must be past cooloff period
    require(mapPID_finalising[currentProposal] == true, "!finalising"); // Must be in finalising stage
    if(!hasQuorum(currentProposal)){
        mapPID_finalising[currentProposal] = false; // If proposal has lost quorum consensus; kick it out of the finalising stage
    } else {
        bytes memory _type = bytes(mapPID_type[currentProposal]); // Get the proposal type
        if(isEqual(_type, 'DAO')){
            moveDao(currentProposal);
        } else if (isEqual(_type, 'ROUTER')) {
            moveRouter(currentProposal);
        } else if (isEqual(_type, 'UTILS')){
            moveUtils(currentProposal);
        } else if (isEqual(_type, 'RESERVE')){
            moveReserve(currentProposal);
        } else if (isEqual(_type, 'FLIP_EMISSIONS')){
            flipEmissions(currentProposal);
        } else if (isEqual(_type, 'COOL_OFF')){
            changeCooloff(currentProposal);
        } else if (isEqual(_type, 'ERAS_TO_EARN')){
            changeEras(currentProposal);
        } else if (isEqual(_type, 'GRANT')){
            grantFunds(currentProposal);
        } else if (isEqual(_type, 'GET_SPARTA')){
            _increaseSpartaAllocation(currentProposal);
        } else if (isEqual(_type, 'LIST_BOND')){
            _listBondingAsset(currentProposal);
        } else if (isEqual(_type, 'DELIST_BOND')){
            _delistBondingAsset(currentProposal);
        } else if (isEqual(_type, 'ADD_CURATED_POOL')){
            _addCuratedPool(currentProposal);
        } else if (isEqual(_type, 'REMOVE_CURATED_POOL')){
            _removeCuratedPool(currentProposal);
        } 
    }
}

function completeProposal(uint _proposalID) internal { string memory _typeStr = mapPID_type[_proposalID]; // Get proposal type emit FinalisedProposal(msg.sender, _proposalID, mapPID_votes[_proposalID], _DAOVAULT.totalWeight(), _typeStr); mapPID_votes[_proposalID] = 0; // Reset proposal votes to 0 mapPID_finalised[_proposalID] = true; // Finalise the proposal mapPID_finalising[_proposalID] = false; // Remove proposal from 'finalising' stage mapPID_open[_proposalID] = false; // Close the proposal }

Tools Used

Recommended Mitigation Steps

Allow multiple simultaneous proposals Make it easier to cancel proposals Require a stake to enter proposals (which might be slashed)

SamusElderg commented 3 years ago

Duplicate of #43 Worth noting the differences though; also the severity is quite low due to being able to proactively or reactively change the DaoFee

ghoul-sol commented 3 years ago

Duplicate of #43

code-423n4 / 2021-07-spartan-findings