hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Application can `suffer DOS` and stop working if Krelos add a bounty to protect against spam #91

Open hats-bug-reporter[bot] opened 3 weeks ago

hats-bug-reporter[bot] commented 3 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xaf56c62c95bf3b08112c0eabb0fb0789581b56385ba8db8ce64d7bd99b83624e Severity: high

Description:

Context

The current flow when creating a categorical market is as follow:

MarketFactory |--> createCategoricalMarket --> createMarket --> createNewMarketParams --> askRealityQuestion  
                                                                                                                                                                                           \ 
                                                                                                                                                                                            \-> Realitio|--> askQuestionWithMinBond --> _askQuestion

Sample deployment Gnosis Chain

MarketFactory has an arbitrator field which is set to 0x29F39dE98D750eb77b5FAfb31B2837f079FcE222 which seem to be Krelos, which we can assume would be the same for real deployement. This field is immutable in the contract, so can't be changed in the future.

Impact

Kleros contract is NOT controlled by Seer team, and as such, at any moment they can decide to call realitio.setQuestionFee to set a bounty to protect against spam, which would effectivelly DOS the Seer app, as not able to create market anymore, it would always revert as Seer is not providing any bounty when creating markets (questions).

Here we can see where Kleros can freely call this any time they desire in Realitio.

    /// @notice Function for arbitrator to set an optional per-question fee. 
    /// @dev The per-question fee, charged when a question is asked, is intended as an anti-spam measure.
    /// @param fee The fee to be charged by the arbitrator when a question is asked
    function setQuestionFee(uint256 fee) 
        stateAny() 
    external {
        arbitrator_question_fees[msg.sender] = fee;
        emit LogSetQuestionFee(msg.sender, fee);
    }

Here we can see the exact location in the creation market where is the problematic situation.

    function _askQuestion(bytes32 question_id, bytes32 content_hash, address arbitrator, uint32 timeout, uint32 opening_ts, uint256 min_bond) 
        stateNotCreated(question_id)
    internal {

        ...

        uint256 bounty = msg.value;

        // The arbitrator can set a fee for asking a question. 
        // This is intended as an anti-spam defence.
        // The fee is waived if the arbitrator is asking the question.
        // This allows them to set an impossibly high fee and make users proxy the question through them.
        // This would allow more sophisticated pricing, question whitelisting etc.
        if (arbitrator != NULL_ADDRESS && msg.sender != arbitrator) {
            uint256 question_fee = arbitrator_question_fees[arbitrator];
            require(bounty >= question_fee, "ETH provided must cover question fee");                      // <<------------- THIS would always revert!
            bounty = bounty - question_fee;
            balanceOf[arbitrator] = balanceOf[arbitrator] + question_fee;
        }

        ...

Recommendation

Refactor to include a bounty in your contracts and pass it along to Realitio

dontonka commented 3 weeks ago

Be aware that while I give example of categorical market, this affects all markets.

xyzseer commented 3 weeks ago

realitio.setQuestionFee must be called by 0x29F39dE98D750eb77b5FAfb31B2837f079FcE222 itself (aka RealitioHomeArbitrationProxy), and the contract doesn't have a function to do it

dontonka commented 3 weeks ago

Good point, I missed that.

On which chain(s) this will be deployed and could you list me the corresponding arbitrator contract on each chain?

clesaege commented 3 weeks ago

Yeah, the proxy can't do it and the arbitrator is trusted. If the arbitrator were to be compromised, the main issue would not be not to be able to create new markets but for the arbitrator to report wrong results. Note that the arbitrator is a decentralized system operating without bugs for 6 years, so it should be pretty safe.