search

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

unsafe type casting during oracle fetching/finalization #136

Open hats-bug-reporter[bot] opened 5 days ago

hats-bug-reporter[bot] commented 5 days ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x47589bae62963903162ca8d2f4c437c889135d38e9bcf7e29b3dfec6810c06f9 Severity: medium

Description: Description\ Seer uses Reality.eth oracle for resolving market outcomes. the RealityProxy.sol is responsible for resolving markets based on the results provided by the Reality.eth oracle. here resolve() function in RealityProxy.sol Resolves the specified market

function resolve(Market market) external {
        bytes32[] memory questionsIds = market.questionsIds();
        uint256 numOutcomes = market.numOutcomes();
        uint256 templateId = market.templateId();
        uint256 low = market.lowerBound();
        uint256 high = market.upperBound();

        // questionId must be a hash of all the values used to resolve a market, this way if an attacker tries to resolve a fake market by changing some value its questionId will not match the id of a valid market.
        bytes32 questionId = keccak256(abi.encode(questionsIds, numOutcomes, templateId, low, high));

        if (templateId == REALITY_SINGLE_SELECT_TEMPLATE) {
            resolveCategoricalMarket(questionId, questionsIds, numOutcomes);//@audit
            return;
        }

        if (templateId == REALITY_MULTI_SELECT_TEMPLATE) {
            resolveMultiCategoricalMarket(questionId, questionsIds, numOutcomes);//@audit
            return;
        }

        if (questionsIds.length > 1) {
            resolveMultiScalarMarket(questionId, questionsIds, numOutcomes);//@audit
            return;
        }

        resolveScalarMarket(questionId, questionsIds, low, high);
    }

the resolve function calls the resolveCategoricalMarket() , resolveMultiCategoricalMarket() and resolveMultiScalarMarket() functions internally and this function uses/calls the realitio for the answer as we can see below 

uint256 answer = uint256(realitio.resultForOnceSettled(questionsIds[0]));
function resolveCategoricalMarket(
        bytes32 questionId,
        bytes32[] memory questionsIds,
        uint256 numOutcomes
    ) internal {
        uint256 answer = uint256(realitio.resultForOnceSettled(questionsIds[0]));//@audit-force typecasting
        uint256[] memory payouts = new uint256[](numOutcomes + 1);

        if (answer == uint256(INVALID_RESULT) || answer >= numOutcomes) {
            // the last outcome is INVALID_RESULT.
            payouts[numOutcomes] = 1;
        } else {
            payouts[answer] = 1;
        }

        conditionalTokens.reportPayouts(questionId, payouts);
    }

but here the function assumes/expects that the resultForOnceSettled returns uint256 but the function actually returns bytes32

function resultForOnceSettled(bytes32 question_id) external view returns (bytes32);

.i.e here the function directly(force) cast the return value(bytes32) to uint256.this issue can lead to unexpected outcomes and it completly depends on the questionId's .the function indeed checks for if (answer == uint256(INVALID_RESULT) || answer >= numOutcomes) i.e for invalid result but this will not enough as mentioned in the dxdao report i.e users submitting answers to oracle questions may not have any knowledge of the internal workings of Carrot KPI contracts.

i am digging deep in this issue for high impact,i will comment below if i find any leads.

below is the same issue found in dxdao carrot-kpi contracts

issue id: CAR-01

https://github.com/sigp/public-audits/blob/master/reports/dxdao/carrot-kpi/review.pdf

https://github.com/carrot-kpi/contracts/commit/a642398b5d71aaf55f9c52ba3632d3fb30865855

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

    function resolve(Market market) external {
    bytes32[] memory questionsIds = market.questionsIds();
    uint256 numOutcomes = market.numOutcomes();
    uint256 templateId = market.templateId();
    uint256 low = market.lowerBound();
    uint256 high = market.upperBound();

    // questionId must be a hash of all the values used to resolve a market, this way if an attacker tries to resolve a fake market by changing some value its questionId will not match the id of a valid market.
    bytes32 questionId = keccak256(abi.encode(questionsIds, numOutcomes, templateId, low, high));

    if (templateId == REALITY_SINGLE_SELECT_TEMPLATE) {
        resolveCategoricalMarket(questionId, questionsIds, numOutcomes);
        return;
    }

    if (templateId == REALITY_MULTI_SELECT_TEMPLATE) {
        resolveMultiCategoricalMarket(questionId, questionsIds, numOutcomes);
        return;
    }

    if (questionsIds.length > 1) {
        resolveMultiScalarMarket(questionId, questionsIds, numOutcomes);//@audit
        return;
    }

    resolveScalarMarket(questionId, questionsIds, low, high);
}
    function resolveCategoricalMarket(
    bytes32 questionId,
    bytes32[] memory questionsIds,
    uint256 numOutcomes
) internal {
    uint256 answer = uint256(realitio.resultForOnceSettled(questionsIds[0]));//@audit-force typecasting
    uint256[] memory payouts = new uint256[](numOutcomes + 1);

    if (answer == uint256(INVALID_RESULT) || answer >= numOutcomes) {
        // the last outcome is INVALID_RESULT.
        payouts[numOutcomes] = 1;
    } else {
        payouts[answer] = 1;
    }

    conditionalTokens.reportPayouts(questionId, payouts);
}

  2. Revised Code File (Optional)

    will provide in the comments

clesaege commented 5 days ago

Similar to https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/97