Core Contracts

Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Submission Details
Severity: medium
Valid

Missing sanity checks on request-id in `fulfillRequest` could allow older requests to be fulfilled

Summary

Missing sanity checks on request-id in fulfillRequest could allow older requests to be fulfilled and overwrite the current price data leading to inaccurate prices for the tokens.

Vulnerability Details

In the documentation for Chainlink Functions, when sendRequest is called, the current request-id is stored as s_lastRequestId:

function sendRequest(
string memory source,
bytes memory encryptedSecretsUrls,
uint8 donHostedSecretsSlotID,
uint64 donHostedSecretsVersion,
string[] memory args,
bytes[] memory bytesArgs,
uint64 subscriptionId,
uint32 gasLimit,
bytes32 donID
) external onlyOwner returns (bytes32 requestId) {
FunctionsRequest.Request memory req;
req.initializeRequestForInlineJavaScript(source);
if (encryptedSecretsUrls.length > 0) {
req.addSecretsReference(encryptedSecretsUrls);
} else if (donHostedSecretsVersion > 0) {
req.addDONHostedSecrets(donHostedSecretsSlotID, donHostedSecretsVersion);
}
if (args.length > 0) req.setArgs(args);
if (bytesArgs.length > 0) req.setBytesArgs(bytesArgs);
s_lastRequestId = _sendRequest(req.encodeCBOR(), subscriptionId, gasLimit, donID);
return s_lastRequestId;
}

This request is fulfilled when fulfillRequest is called by Chainlink:

function fulfillRequest(bytes32 requestId, bytes memory response, bytes memory err) internal override {
if (s_lastRequestId != requestId) {
revert UnexpectedRequestID(requestId);
}
s_lastResponse = response;
s_lastError = err;
emit Response(requestId, s_lastResponse, s_lastError);
}

There is this check on requestId which checks whether it equals the s_lastRequestId:

if (s_lastRequestId != requestId) {
revert UnexpectedRequestID(requestId);
}

The check ensures that only the latest request is fulfilled, preventing outdated responses from being accepted. Since Chainlink requests are asynchronous, multiple requests can be in flight simultaneously.

Without this check, an older request could overwrite a newer response, leading to inconsistent or incorrect contract behaviour. This safeguard ensures single-request consistency. Have a look at the link below:

https://ethereum.stackexchange.com/questions/168003/what-will-be-the-consequences-of-not-having-sanity-checks-on-requestid-when

But in the abstract contract BaseChainlinkFunctionsOraclewhich is inherited by both RAACPrimeRateOracle and RAACHousePriceOracle:

function fulfillRequest(
bytes32 requestId,
bytes memory response,
bytes memory err
) internal override {
s_lastResponse = response;
s_lastError = err;
if (err.length == 0) {
if (response.length == 0) {
revert FulfillmentFailed();
}
_processResponse(response);
}
}

The above check is missing. This can lead to inconsistent price updates.

Impact

An older request could overwrite a newer response, leading to inconsistent or incorrect updates to the price of the tokens.

Tools Used

Manual review

Recommendations

As recommended by Chainlink docs, use the above check for request-id.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Oracle Race Condition in RAACHousePriceOracle causes price misassignment between NFTs

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Oracle Race Condition in RAACHousePriceOracle causes price misassignment between NFTs

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.