Weather Witness

First Flight #40
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Potential DoS with Failed Transfers

Summary

The LINK token transfer in the requestMintWeatherNFT function could fail silently, potentially leading to a denial of service condition where users pay ETH but the automation registration fails.

Vulnerability Details

In the requestMintWeatherNFT function, the contract attempts to transfer LINK tokens from the user but doesn't properly handle potential transfer failures:

if (_registerKeeper) {
IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
);
}

While the contract uses safeTransferFrom, which reverts on failure, there's no specific error handling or recovery mechanism if this fails after the user has already paid ETH.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;
import "forge-std/Test.sol";
import "../src/WeatherNft.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract FailedTransferTest is Test {
WeatherNft public weatherNft;
address public user;
IERC20 public linkToken;
function setUp() public {
// Setup the WeatherNft contract and necessary dependencies
// This would include deploying the contract and setting up any required configurations
// For this PoC, we assume weatherNft is already deployed
weatherNft = WeatherNft(address(0x123)); // Replace with actual address
linkToken = IERC20(address(0x456)); // Replace with LINK token address
user = address(0x789); // User address
// Fund the user with ETH but not enough LINK
vm.deal(user, 10 ether);
// Give the user some LINK, but not enough for the deposit
deal(address(linkToken), user, 0.01 ether);
}
function testInsufficientLinkAllowance() public {
vm.startPrank(user);
// Get the current mint price
uint256 mintPrice = weatherNft.s_currentMintPrice();
// User approves LINK tokens, but not enough
linkToken.approve(address(weatherNft), 0.01 ether);
// User attempts to mint with keeper registration
// This will fail because the user hasn't approved enough LINK tokens
try weatherNft.requestMintWeatherNFT{value: mintPrice}(
"12345",
"US",
true, // Register keeper
3600,
0.1 ether // More than the user has approved
) {
fail("Transaction should have reverted");
} catch {
// Transaction reverted as expected
// But the user has already sent ETH which is now locked
}
// Check that the user's ETH is now locked in the contract
// and they have no NFT or automation
vm.stopPrank();
}
function testInsufficientLinkBalance() public {
vm.startPrank(user);
// Get the current mint price
uint256 mintPrice = weatherNft.s_currentMintPrice();
// User approves LINK tokens
linkToken.approve(address(weatherNft), 1 ether); // Approve more than they have
// User attempts to mint with keeper registration
// This will fail because the user doesn't have enough LINK tokens
try weatherNft.requestMintWeatherNFT{value: mintPrice}(
"12345",
"US",
true, // Register keeper
3600,
0.1 ether // More than the user has
) {
fail("Transaction should have reverted");
} catch {
// Transaction reverted as expected
// But the user has already sent ETH which is now locked
}
// Check that the user's ETH is now locked in the contract
// and they have no NFT or automation
vm.stopPrank();
}
function testLinkTransferFailure() public {
// Create a mock LINK token that fails on transfers
MockFailingLinkToken mockLink = new MockFailingLinkToken();
// Update the contract to use the mock LINK token
// (This would require a function to update the LINK token address)
// weatherNft.updateLinkToken(address(mockLink));
vm.startPrank(user);
// Get the current mint price
uint256 mintPrice = weatherNft.s_currentMintPrice();
// User approves LINK tokens
mockLink.approve(address(weatherNft), 1 ether);
// User attempts to mint with keeper registration
// This will fail because the LINK token transfer will fail
try weatherNft.requestMintWeatherNFT{value: mintPrice}(
"12345",
"US",
true, // Register keeper
3600,
0.1 ether
) {
fail("Transaction should have reverted");
} catch {
// Transaction reverted as expected
// But the user has already sent ETH which is now locked
}
vm.stopPrank();
}
}
// Mock LINK token that fails on transfers
contract MockFailingLinkToken is IERC20 {
mapping(address => uint256) private _balances;
mapping(address => mapping(address => uint256)) private _allowances;
function approve(address spender, uint256 amount) external returns (bool) {
_allowances[msg.sender][spender] = amount;
return true;
}
function transferFrom(address sender, address recipient, uint256 amount) external returns (bool) {
// Always fail on transferFrom
revert("MockLINK: transfer failed");
}
// Implement other IERC20 functions as needed
function totalSupply() external view returns (uint256) { return 0; }
function balanceOf(address account) external view returns (uint256) { return _balances[account]; }
function transfer(address recipient, uint256 amount) external returns (bool) { return false; }
function allowance(address owner, address spender) external view returns (uint256) { return _allowances[owner][spender]; }
}

This PoC demonstrates three scenarios where the LINK token transfer could fail:

  1. Insufficient allowance: The user approves less LINK than required

  2. Insufficient balance: The user has less LINK than required

  3. Transfer failure: The LINK token itself fails during the transfer operation

In all cases, the user's ETH payment would be processed before the LINK transfer fails, potentially leaving them with locked funds and no NFT or automation.

Impact

If the LINK token transfer fails (due to insufficient allowance, balance issues, or token contract problems):

  • The user will have already paid ETH for minting

  • The automation registration will fail

  • The user may be left with an NFT that doesn't have automation set up

  • This creates a poor user experience and potential financial loss


Recommendations

Implement proper error handling and transaction management:

  1. Use a try-catch pattern to handle transfer failures gracefully:

if (_registerKeeper) {
try IERC20(s_link).safeTransferFrom(
msg.sender,
address(this),
_initLinkDeposit
) {
// Continue with keeper registration
} catch {
// Handle failure - either revert the whole transaction or set a flag
revert("WeatherNft__LinkTransferFailed");
}
}
  1. Consider implementing a two-step process where users first approve LINK tokens, then a separate transaction handles the ETH payment and minting.

  2. Add a mechanism for users to retry automation registration if it fails initially.

Updates

Appeal created

bube Lead Judge 5 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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