Submission Details
Severity:
-weather-witness
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
// Root cause in the codebase with @> marks to highlight the relevant section// Root cause in the codebase with @> marks to highlight the relevant Root Causes with Exploitation Scenarios
Risk
Likelihood:
-
Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
-
Reason 2
Impact:
-
Impact 1
-
Impact 2
Proof of Concept
Introduction
This document outlines the identified vulnerabilities in the Weather NFT contract and demonstrates how they could be exploited. The vulnerabilities range from critical security issues to minor optimizations.
1. Front-Running Attack in fulfillMintRequest
Vulnerability
The fulfillMintRequest function didn't verify that the caller was the original requester, allowing anyone to front-run the fulfillment.
PoC Exploit
// Attacker actions
function attackFulfillMint(bytes32 targetRequestId) external {
// Monitor mempool for oracle responses
// When oracle fulfills a mint request with a valuable response
// Front-run the legitimate user's transaction
weatherNft.fulfillMintRequest(targetRequestId);
// Attacker now owns the NFT instead of legitimate user
}
Fix Implemented
function fulfillMintRequest(bytes32 requestId) external nonReentrant {
// ...
UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
require(_userMintRequest.user == msg.sender, WeatherNft__Unauthorized());
// ...
}
2. Uncapped Mint Price
Vulnerability
The mint price increased without any upper bound, potentially causing the contract to become unusable over time.
PoC Exploit
// After many mints (simulation)
function showMintPriceExplosion() external {
for(uint i = 0; i < 1000; i++) {
// Simulate mints
weatherNft.s_currentMintPrice += weatherNft.s_stepIncreasePerMint;
}
// Price becomes excessively high
console.log("Current mint price:", weatherNft.s_currentMintPrice);
// Result: Prohibitively expensive for users
}
Fix Implemented
uint256 public constant MAX_MINT_PRICE = 10 ether;
// In requestMintWeatherNFT function:
uint256 newPrice = s_currentMintPrice + s_stepIncreasePerMint;
s_currentMintPrice = newPrice > MAX_MINT_PRICE ? MAX_MINT_PRICE : newPrice;
The GetWeather.js used an insecure HTTP endpoint rather than HTTPS, making it susceptible to man-in-the-middle attacks.
PoC Exploit
// Man-in-the-middle attack scenario
async function interceptWeatherData() {
// Set up interception on unencrypted connection
// When weather API is called via HTTP
// Intercept and modify the weather data
// User receives manipulated weather data
return {
id: 800, // Manipulated to always report SUNNY
main: "Clear",
description: "Manipulated data"
};
}
const geoCodingRequest = Functions.makeHttpRequest({
url: "https://api.openweathermap.org/geo/1.0/zip", // Now using HTTPS
// ...
})
4. Reentrancy Vulnerability
// Malicious contract
contract Attacker {
WeatherNft target;
constructor(address _target) {
target = WeatherNft(_target);
}
// Malicious callback
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
// Make reentrant call
// ...
return IERC721Receiver.onERC721Received.selector;
}
}
import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface, ReentrancyGuard {
// Added nonReentrant modifier to key functions
function fulfillMintRequest(bytes32 requestId) external nonReentrant {
// ...
}
function performUpkeep(bytes calldata performData) external override nonReentrant {
// ...
}
}
5. Locked ETH
// Simulate accumulation of ETH
function demonstrateLockedFunds() external {
// After many mints
uint256 contractBalance = address(weatherNft).balance;
console.log("Locked ETH in contract:", contractBalance);
// No way to extract these funds
}
function withdrawFunds() external onlyOwner {
uint256 balance = address(this).balance;
require(balance > 0, WeatherNft__NoFundsToWithdraw());
(bool success, ) = payable(owner()).call{value: balance}("");
require(success, WeatherNft__WithdrawalFailed());
emit FundsWithdrawn(balance);
}
6. Lack of Return Value Checks
// Demonstrate failure when token approval fails
function demonstrateMissingApprovalCheck() external {
// Mock a scenario where approve returns false
// Function continues as if approval succeeded
// Later operations fail unexpectedly
}
Fix Implemented
bool approved = LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
require(approved, WeatherNft__ApprovalFailed());
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep(_keeperParams);
require(upkeepId > 0, WeatherNft__UpkeepRegistrationFailed());
7. Unchecked Token Existence
// Attempt to update non-existent token
function demonstrateNonExistentTokenUpdate() external {
uint256 nonExistentTokenId = 9999;
// Before fix: No error, wasted gas, potential undefined behavior
weatherNft.performUpkeep(abi.encode(nonExistentTokenId));
}
Recommended Mitigation
- - UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
uint8 weather = abi.decode(response, (uint8));this code
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());
+ // Ensure the caller is the original requester
+ UserMintRequest memory _userMintRequest = s_funcReqIdToUserMintReq[requestId];
+ require(_userMintRequest.user == msg.sender, WeatherNft__Unauthorized());
if (response.length == 0 || err.length > 0) {
+ emit MintRequestFailed(requestId, msg.sender, err);
return;
}
// ...
}+ import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
- contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface {
+ contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface, ReentrancyGuard {
// ...
- function fulfillMintRequest(bytes32 requestId) external {
+ function fulfillMintRequest(bytes32 requestId) external nonReentrant {
// ...
}
- function performUpkeep(bytes calldata performData) external override {
+ function performUpkeep(bytes calldata performData) external override nonReentrant {
// ...
}
}const geoCodingRequest = Functions.makeHttpRequest({
- url: "http://api.openweathermap.org/geo/1.0/zip",
+ url: "https://api.openweathermap.org/geo/1.0/zip",
method: "GET",
params: { zip: `${args[0]},${args[1]}`, appid: secrets.apiKey }
})+ add this code+ // Maximum mint price to prevent runaway pricing
+ uint256 public constant MAX_MINT_PRICE = 10 ether;
function requestMintWeatherNFT(...) external payable nonReentrant returns (bytes32 _reqId) {
// ...
require(msg.value == s_currentMintPrice, WeatherNft__InvalidAmountSent());
- s_currentMintPrice += s_stepIncreasePerMint;
+ // Cap the mint price to prevent runaway prices
+ uint256 newPrice = s_currentMintPrice + s_stepIncreasePerMint;
+ s_currentMintPrice = newPrice > MAX_MINT_PRICE ? MAX_MINT_PRICE : newPrice;
// ...
}constructor(...) {
require(weathers.length == weatherURIs.length, WeatherNft__IncorrectLength());
+ // Validate URIs are not empty
for (uint256 i; i < weathers.length; ++i) {
+ require(bytes(weatherURIs[i]).length > 0, WeatherNft__InvalidURI());
s_weatherToTokenURI[weathers[i]] = weatherURIs[i];
}
// ...
}
if (_userMintRequest.registerKeeper) {
// Register chainlink keeper to pull weather data in order to automate weather nft
- LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
+ bool approved = LinkTokenInterface(s_link).approve(s_keeperRegistrar, _userMintRequest.initLinkDeposit);
+ require(approved, WeatherNft__ApprovalFailed());
// ...
upkeepId = IAutomationRegistrarInterface(s_keeperRegistrar).registerUpkeep(_keeperParams);
+ require(upkeepId > 0, WeatherNft__UpkeepRegistrationFailed());
}+ // Function to withdraw accumulated ETH
+ function withdrawFunds() external onlyOwner {
+ uint256 balance = address(this).balance;
+ require(balance > 0, WeatherNft__NoFundsToWithdraw());
+
+ (bool success, ) = payable(owner()).call{value: balance}("");
+ require(success, WeatherNft__WithdrawalFailed());
+
+ emit FundsWithdrawn(balance);
+ }function tokenURI(uint256 tokenId) public view override returns (string memory) {
_requireOwned(tokenId);
string memory image = s_weatherToTokenURI[s_tokenIdToWeather[tokenId]];
bytes memory jsonData = abi.encodePacked(
- '{"name": "Weathear NFT", "user": "',
+ '{"name": "Weather NFT", "user": "',
Strings.toHexString(_ownerOf(tokenId)),
'", "image": "',
image, '"}'
);
// ...
}
function performUpkeep(bytes calldata performData) external override nonReentrant {
uint256 _tokenId = abi.decode(performData, (uint256));
+ // Check that token exists
+ require(_ownerOf(_tokenId) != address(0), WeatherNft__TokenDoesNotExist());
uint256 upkeepId = s_weatherNftInfo[_tokenId].upkeepId;
// ...
}
+ uint256 public constant MIN_HEARTBEAT = 1 hours;
+ uint256 public constant MAX_HEARTBEAT = 30 days;
function requestMintWeatherNFT(...) external payable nonReentrant returns (bytes32 _reqId) {
+ // Validate parameters
+ require(bytes(_pincode).length > 0, WeatherNft__InvalidParams());
+ require(bytes(_isoCode).length > 0, WeatherNft__InvalidParams());
+ require(_heartbeat >= MIN_HEARTBEAT && _heartbeat <= MAX_HEARTBEAT, WeatherNft__InvalidHeartbeat());
// ...
if (_registerKeeper) {
+ require(_initLinkDeposit > 0, WeatherNft__InvalidParams());
// ...
}
}
Updates
Appeal created
bube 23 days ago
Submission Judgement Published Assigned finding tags:
Lack of `withdraw` function
The contract collects funds for minting a WeatherNFT, but there is no function that allows the owner to withdraw these funds.
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.
Use of `http` instead of `https` for getting geo location
Rewards breakdown
nSLOC
236This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.