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

Risk of race condition if 2 requests are sent before the first one is fulfilled

infect3d

Summary

The current implementation of sendRequest do not ensure that the previous one has been fulfilled before sending a new request.
If two request Req1 and Req2 are sent before any response is received, this can cause a race condition where the response of Req2 will be written to Req1 houseId

Vulnerability details

BaseChainlinkFunctionsOracle::sendRequest uses the Chainlink Functions to retrieve house prices through API call that are executed by a network of generalized oracles.

When a request is sent on-chain, an event is emitted, which is then caught by the Chainlink network that will execute the request, and return the requested value by calling the BaseChainlinkFunctionsOracle::fulfillRequest function.

During sendRequest, an internal function _beforeFulfill is called:

File: contracts/core/oracles/BaseChainlinkFunctionsOracle.sol
48: function sendRequest(
...:
...: //* --------------- some code --------------- *//
...:
65: if (args.length > 0) {
66: req.setArgs(args);
67: }
...: //* --------------- some code --------------- *//
71: s_lastRequestId = _sendRequest( <@(1) send the request
72: req.encodeCBOR(),
73: subscriptionId,
74: callbackGasLimit,
75: donId
76: );
77: _beforeFulfill(args); <@(2) prepare the fulfillment
78: }

And here's what is done in _beforeFulfill:

File: contracts/core/oracles/RAACHousePriceOracle.sol
34: function _beforeFulfill(string[] calldata args) internal override {
35: lastHouseId = args[0].stringToUint();
36: }

The function sets the lastHouseId to the same value that the one that is encoded in the request.

Then, when Chainlink returns the response by calling fulfillRequest, the internal function _processResponse is called and finally set the price of the lastHouseId

File: contracts/core/oracles/RAACHousePriceOracle.sol
42: function _processResponse(bytes memory response) internal override {
43: uint256 price = abi.decode(response, (uint256));
44: housePrices.setHousePrice(lastHouseId, price);
45: emit HousePriceUpdated(lastHouseId, price);
46: }

Now, if 2 requests are sent, the first one will be for houseId1 and the second for houseId2.
Which means that lastHouseId will be set to houseId2.
But when the fulfillment will be returned for the first request (which is houseId1) , it will be written as the price for houseId2

Impact

As there is no insurance in which order the requests will be processed by the Chainlink network, there is risk of race condition when multiple requests are sent.
If this occurs, the wrong price will be registered for a houseId.

Recommended Mitigation Steps

There are two possible ways to manage this:

  1. Prevent new requests to be sent until the previous one has been fulfilled (e.g. by using a lock variable)

  2. In _processResponse, rather than relying on the internal state variable lastHouseId, make the response also return the requested houseId, and use that value to update the price.

Example for method 2:

function _processResponse(bytes memory response) internal override {
- uint256 price = abi.decode(response, (uint256));
+ (uint256 houseId, uint256 price) = abi.decode(response, (uint256, uint256));
housePrices.setHousePrice(houseId, price);
emit HousePriceUpdated(houseId, price);
}
Updates

Lead Judging Commences

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

Oracle Race Condition in RAACHousePriceOracle causes price misassignment between NFTs

inallhonesty Lead Judge 6 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.