Summary
The contract originally used a single global state variable (lastHouseId) to track which house ID corresponded to an incoming Chainlink Functions request. When multiple requests were sent in quick succession or their responses arrived out of order, data would be written under the wrong house ID, causing out-of-sync or corrupted pricing information.
Vulnerability Details
Cause: All requests overwrote a single global variable (lastHouseId), making it impossible to map each response to the correct request if fulfillments came in non-sequentially.
Result: Inaccurate or misleading data could be stored for the wrong house ID.
Restriction: Only the contract owner can invoke sendRequest. Because of this restriction, the vulnerability could not be directly exploited by external parties, but it could unintentionally disrupt data integrity if the owner makes consecutive requests.
Impact
Data Corruption: Incorrect house prices would be recorded if the fulfillment for one house ID arrived after another request had already overwritten the global variable.
Limited Exploitability: Due to the onlyOwner modifier, external attackers cannot exploit this directly. However, legitimate users (the owner) could inadvertently produce incorrect data on-chain.
Tools Used
Manual Code Review – Identification of the single global variable usage pattern and out-of-order fulfillment risk.
Testing/Simulation – Sending multiple Chainlink requests in quick succession to observe overwriting behavior.
Recommendations
Use a Mapping: Store a mapping from requestId to houseId, ensuring each request is correctly tied to the appropriate house data.
Retrieve Data by requestId: In the fulfillment function, look up the houseId from the requestId-based mapping before writing the final price.
Validate Args: Check that the array of arguments passed to sendRequest has the expected length and format to avoid invalid indexing in _beforeFulfill.
Modifications to the BaseChainlinkFunctionsOracle.sol file:
/**
* @notice Triggers an on-demand Functions request using remote encrypted secrets
* @param source JavaScript source code
* @param secretsLocation Location of secrets (only Location.Remote & Location.DONHosted are supported)
* @param encryptedSecretsReference Reference pointing to encrypted secrets
* @param args String arguments passed into the source code and accessible via the global variable `args`
* @param bytesArgs Bytes arguments passed into the source code and accessible via the global variable `bytesArgs` as hex strings
* @param subscriptionId Subscription ID used to pay for request (FunctionsConsumer contract address must first be added to the subscription)
* @param callbackGasLimit Maximum amount of gas used to call the inherited `handleOracleFulfillment` method
*/
function sendRequest(
string calldata source,
FunctionsRequest.Location secretsLocation,
bytes calldata encryptedSecretsReference,
string[] calldata args,
bytes[] calldata bytesArgs,
uint64 subscriptionId,
uint32 callbackGasLimit
) external onlyOwner {
FunctionsRequest.Request memory req;
req.initializeRequest(
FunctionsRequest.Location.Inline,
FunctionsRequest.CodeLanguage.JavaScript,
source
);
req.secretsLocation = secretsLocation;
req.encryptedSecretsReference = encryptedSecretsReference;
if (args.length > 0) {
req.setArgs(args);
}
if (bytesArgs.length > 0) {
req.setBytesArgs(bytesArgs);
}
s_lastRequestId = _sendRequest(
req.encodeCBOR(),
subscriptionId,
callbackGasLimit,
donId
);
- _beforeFulfill(args);
+ _beforeFulfill(s_lastRequestId, args);
}
/**
* @notice Hook that is called before fulfillment with the request arguments
* @param args The arguments that were passed to sendRequest
*/
- function _beforeFulfill(string[] calldata args) internal virtual;
+ function _beforeFulfill(bytes32 requestId, string[] calldata args) internal virtual;
/**
* @notice Hook that is called to process the response
* @param response The response from the oracle
*/
- function _processResponse(bytes memory response) internal virtual;
+ function _processResponse(bytes32 requestId, bytes memory response) internal virtual;
/**
* @notice Store latest result/error
* @param requestId The request ID, returned by sendRequest()
* @param response Aggregated response from the user code
* @param err Aggregated error from the user code or from the execution pipeline
* Either response or error parameter will be set, but never both
*/
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);
+ _processResponse(requestId, response);
}
}
/**
* @title RAACHousePriceOracle
* @dev Contract for using oracles to fetch house pricing data from off-chain api
* This contract allows an oracle to update prices.
*/
contract RAACHousePriceOracle is BaseChainlinkFunctionsOracle {
using StringUtils for string;
RAACHousePrices public housePrices;
- uint256 public lastHouseId;
+ /// @notice Maps Chainlink request IDs to their corresponding house IDs
+ /// @dev Used to correlate oracle responses with specific houses when processing
+ /// price updates in the _processResponse function
+ /// @custom:key bytes32 The unique request ID generated by Chainlink
+ /// @custom:value uint256 The ID of the house for which the price update was requested
+ mapping(bytes32 requestId => uint256 houseId) public requestIdToHouseId;
event HousePriceUpdated(uint256 indexed houseId, uint256 price);
constructor(
address router,
bytes32 _donId,
address housePricesAddress
) BaseChainlinkFunctionsOracle(router, _donId) {
require(housePricesAddress != address(0), "HousePrices address must be set");
housePrices = RAACHousePrices(housePricesAddress);
}
/**
* @notice Hook called before fulfillment to store the house ID
* @param args The arguments passed to sendRequest
*/
- function _beforeFulfill(string[] calldata args) internal override {
- lastHouseId = args[0].stringToUint();
- }
+ function _beforeFulfill(bytes32 requestId, string[] calldata args) internal override {
+ uint256 houseIdOfRequest = args[0].stringToUint();
+ requestIdToHouseId[requestId] = houseIdOfRequest;
+ }
/**
* @notice Process the response from the oracle
* @param response The response from the oracle
*/
- function _processResponse(bytes memory response) internal override {
- uint256 price = abi.decode(response, (uint256));
- housePrices.setHousePrice(lastHouseId, price);
- emit HousePriceUpdated(lastHouseId, price);
- }
+ function _processResponse(bytes32 requestId, bytes memory response) internal override {
+ uint256 price = abi.decode(response, (uint256));
+ uint256 houseId = requestIdToHouseId[requestId];
+ housePrices.setHousePrice(houseId, price);
+ emit HousePriceUpdated(houseId, price);
+ }
}
