Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

`BaseChainlinkFunctionsOracle::fulfillRequest` Should Not Revert & Is Missing Error Handling With Proper State Management

0xdarko

Summary

BaseChainlinkFunctionsOracle::fulfillRequest function contains critical flaws in its error handling and state management. The function can revert under certain conditions, which is problematic since Chainlink Functions DON does not retry failed fulfillments. Additionally, the function lacks proper request ID validation and event emissions for tracking state changes.

Vulnerability Details

[](https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/oracles/BaseChainlinkFunctionsOracle.sol#L99-L112)

  1. Reversion on Empty Response:

if (err.length == 0) {
if (response.length == 0) {
revert FulfillmentFailed(); // Problematic reversion
}
_processResponse(response);
}

The function reverts if there's no error but an empty response, which can lead to permanent state lock.

  1. Missing Request ID Validation:

function fulfillRequest(
bytes32 requestId,
bytes memory response,
bytes memory err
) internal override {
s_lastResponse = response;
s_lastError = err;
// No validation of requestId against s_lastRequestId

The function fails to verify that the incoming requestId matches s_lastRequestId, potentially processing responses from unexpected requests.

  1. Unsafe External Call:

_processResponse(response); // Unsafe external call without try/catch

The call to _processResponse lacks error handling, which could cause the entire fulfillment to revert.

  1. Missing Event Emissions:
    The contract lacks events for tracking response fulfillment and error states, reducing transparency and making it difficult to monitor the contract's state changes off-chain.

Also refer to Cyfrins report on this issue with fulfillRequest

Impact

  • Complete DoS of the oracle functionality if a fulfillment reverts, as s_lastRequestId will not be reset

  • Processing of invalid or unexpected responses due to missing request ID validation

  • Permanent state lock requiring manual intervention

  • Limited ability to monitor and debug issues due to lack of event emissions

Tools Used

Manual Review

Recommendations

Implement Non-Reverting Error Handling & State Resetting:

function fulfillRequest(
bytes32 requestId,
bytes memory response,
bytes memory err
) internal override {
+ if (s_lastRequestId != requestId) {
+ emit UnexpectedRequestID(requestId);
+ s_lastRequestId = bytes32(0);
+ return;
+ }
s_lastResponse = response;
s_lastError = err;
​
- if (err.length == 0) {
- if (response.length == 0) {
- revert FulfillmentFailed();
- }
​
+ if (response.length == 0 && err.length == 0) {
+ emit ProcessingError(requestId);
+ } else {
+ try {
+ _processResponse(response);
+ emit Response(requestId, response, err);
+ } catch {
+ emit ProcessingError(requestId);
+ }
+ }
​
+ s_lastRequestId = bytes32(0);
}

  1. Add Required Events:

event Response(bytes32 indexed requestId, bytes response, bytes error);
event UnexpectedRequestID(bytes32 requestId);
event ProcessingError(bytes32 indexed requestId);
Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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