code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L107-L157 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L620-L631

Vulnerability details

Impact

In this code, a trusted node can submit data several times.

The trusted node can submit ExchangeRateData and then it can submit different data again about same reportingBlockNumber.

This will occur mess of staderOracle contract, so it will be needed to be checked.

Proof of Concept

For example, in the submitExchangeRateData function of staderOracle, if a trusted node submits the ExchangeRate multiple times, the ExchangeRate will be saved in Stader Oracle.

However, it becomes challenging to determine which data is correct.

It's all because, oracle don't check last submmited reportingBlockNumber by certain trust node.

It is important for Stader Oracle contract to check all cases.

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L107-L157

function submitExchangeRateData(ExchangeRate calldata _exchangeRate)
    external
    override
    trustedNodeOnly
    checkMinTrustedNodes
    checkERInspectionMode
    whenNotPaused
{
    if (isPORFeedBasedERData) {
        revert InvalidERDataSource();
    }
    if (_exchangeRate.reportingBlockNumber >= block.number) {
        revert ReportingFutureBlockData();
    }
    if (_exchangeRate.reportingBlockNumber % updateFrequencyMap[ETHX_ER_UF] > 0) {
        revert InvalidReportingBlock();
    }

    // Get submission keys
    bytes32 nodeSubmissionKey = keccak256(
        abi.encode(
            msg.sender,
            _exchangeRate.reportingBlockNumber,
            _exchangeRate.totalETHBalance,
            _exchangeRate.totalETHXSupply
        )
    );
    bytes32 submissionCountKey = keccak256(
        abi.encode(_exchangeRate.reportingBlockNumber, _exchangeRate.totalETHBalance, _exchangeRate.totalETHXSupply)
    );
    uint8 submissionCount = attestSubmission(nodeSubmissionKey, submissionCountKey);
    // Emit balances submitted event
    emit ExchangeRateSubmitted(
        msg.sender,
        _exchangeRate.reportingBlockNumber,
        _exchangeRate.totalETHBalance,
        _exchangeRate.totalETHXSupply,
        block.timestamp
    );

    if (
        submissionCount == trustedNodesCount / 2 + 1 &&
        _exchangeRate.reportingBlockNumber > exchangeRate.reportingBlockNumber
    ) {
        updateWithInLimitER(
            _exchangeRate.totalETHBalance,
            _exchangeRate.totalETHXSupply,
            _exchangeRate.reportingBlockNumber
        );
    }
}

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L620-L631

function attestSubmission(bytes32 _nodeSubmissionKey, bytes32 _submissionCountKey)
    internal
    returns (uint8 _submissionCount)
{
    // Check & update node submission status
    if (nodeSubmissionKeys[_nodeSubmissionKey]) {
        revert DuplicateSubmissionFromNode();
    }
    nodeSubmissionKeys[_nodeSubmissionKey] = true;
    submissionCountKeys[_submissionCountKey]++;
    _submissionCount = submissionCountKeys[_submissionCountKey];
}

Tools Used

Recommended Mitigation Steps

It's necessary to add additional code to check the reported Block number by one trust node.

Assessed type

Invalid Validation

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #44

Picodes commented 1 year ago

What about https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderOracle.sol#L626 which prevent duplicates submissions?