code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

It is possible to create a proposal with an invalid parameter type #974

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L155

Vulnerability details

Impact

It is possible to create a proposal with an invalid parameter type. The parameter type can range from 0 to 24 but when a parameter proposal ballot is created, the parameterType is never validated. Anyone can create an invalid proposal.

    function proposeParameterBallot(
        uint256 parameterType, // [0, 24]
        string calldata description
    ) external nonReentrant returns (uint256 ballotID) {       
        string memory ballotName = string.concat(
            "parameter:",
            Strings.toString(parameterType)
        );

        return
            _possiblyCreateProposal(
                ballotName,
                BallotType.PARAMETER,
                address(0),
                parameterType,
                "",
                description
            );
    }

If the voters decide to vote and finalize the invalid proposal during the calling of the finalizeBallot function, the whole transaction will revert. This is because in the _finalizeParameterBallot function, during the casting to ParameterType, it will return an error.

        if (winningVote == Vote.INCREASE)
            _executeParameterChange(
                ParameterTypes(ballot.number1), // revert
                true,
                poolsConfig,
                stakingConfig,
                rewardsConfig,
                stableConfig,
                daoConfig,
                priceAggregator
            );
        else if (winningVote == Vote.DECREASE)
            _executeParameterChange(
                ParameterTypes(ballot.number1), // revert
                false,
                poolsConfig,
                stakingConfig,
                rewardsConfig,
                stableConfig,
                daoConfig,
                priceAggregator
            );

Proof of Concept

Paste the test in the Proposals.t.sol file.

function testInvalidParameterBallot() public {
        vm.startPrank(DEPLOYER);
        staking.stakeSALT(1000 ether);
        proposals.proposeParameterBallot(999, "description" );
        vm.stopPrank();

    }

Tools Used

Manual Review

Recommended Mitigation Steps

Validate the parameterType before creating the proposal.

Assessed type

DoS

c4-judge commented 8 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 7 months ago

othernet-global (sponsor) confirmed

othernet-global commented 7 months ago

parameterType now validated https://github.com/othernet-global/salty-io/commit/524b59900013d90d17db2b34263c4973a866ab38

Picodes commented 7 months ago

Downgrading this to Low as either it would be a user mistake so this is a safety check and falls within QA, or the user would just harm himself.

c4-judge commented 7 months ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 7 months ago

Picodes marked the issue as grade-c