Weather Witness

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

Centralization Risk in Admin Functions

Summary

The WeatherNft contract has multiple functions restricted to onlyOwner, creating a centralization risk if the owner account is compromised or acts maliciously.

Vulnerability Details

Multiple critical functions are restricted to onlyOwner:

function updateFunctionsGasLimit(uint32 newGasLimit) external onlyOwner {
s_functionsConfig.gasLimit = newGasLimit;
}
function updateSubId(uint64 newSubId) external onlyOwner {
s_functionsConfig.subId = newSubId;
}
function updateSource(string memory newSource) external onlyOwner {
s_functionsConfig.source = newSource;
}
function updateEncryptedSecretsURL(
bytes memory newEncryptedSecretsURL
) external onlyOwner {
s_functionsConfig.encryptedSecretsURL = newEncryptedSecretsURL;
}
function updateKeeperGaslimit(uint32 newGasLimit) external onlyOwner {
s_upkeepGaslimit = newGasLimit;
}

This creates a single point of failure where the owner has excessive control over critical contract parameters.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.29;
import "forge-std/Test.sol";
import "../src/WeatherNft.sol";
contract CentralizationRiskTest is Test {
WeatherNft public weatherNft;
address public owner;
address public attacker;
address public user;
function setUp() public {
// Setup the WeatherNft contract and necessary dependencies
owner = address(0x1);
attacker = address(0x2);
user = address(0x3);
// Deploy the contract with owner as the owner
vm.startPrank(owner);
weatherNft = new WeatherNft(
/* constructor parameters */
);
vm.stopPrank();
// Fund accounts
vm.deal(owner, 10 ether);
vm.deal(attacker, 10 ether);
vm.deal(user, 10 ether);
}
function testCompromisedOwner() public {
// Simulate owner account being compromised by attacker
vm.startPrank(attacker);
// Attempt to compromise the owner account (in a real scenario, this would be through
// phishing, private key theft, etc.)
vm.assume(attacker == owner); // This is a test assumption to simulate compromise
// Now the attacker can perform malicious actions as the owner
// 1. Change the Chainlink Functions source to a malicious one
string memory maliciousSource = "function weather() { return {weather_enum: 0, temp: 0}; }";
weatherNft.updateSource(maliciousSource);
// 2. Set gas limits too low to cause failures
weatherNft.updateFunctionsGasLimit(1000); // Too low to execute properly
weatherNft.updateKeeperGaslimit(1000); // Too low to execute properly
// 3. Update subscription ID to an invalid one
weatherNft.updateSubId(999999); // Non-existent subscription
// 4. Update encrypted secrets URL to a malicious one
bytes memory maliciousSecretsURL = bytes("https://malicious-url.com/secrets");
weatherNft.updateEncryptedSecretsURL(maliciousSecretsURL);
// Verify that the changes were made
// (In a real test, we would need getter functions to verify these changes)
vm.stopPrank();
// Now simulate a user trying to use the system after it's been compromised
vm.startPrank(user);
// User attempts to mint an NFT, but it will fail or behave unexpectedly
// due to the malicious changes made by the attacker
vm.stopPrank();
}
function testNoTimelock() public {
// Demonstrate that critical parameter changes take effect immediately
// without any timelock or governance process
vm.startPrank(owner);
// Record the current gas limit
uint32 originalGasLimit = 100000; // Assume this is the current value
// Change to a new value
uint32 newGasLimit = 50000;
weatherNft.updateFunctionsGasLimit(newGasLimit);
// The change takes effect immediately without any delay or approval process
// In a real test, we would verify this with a getter function
vm.stopPrank();
}
}

This PoC demonstrates how a compromised owner account could manipulate critical contract parameters, potentially causing the system to malfunction or behave maliciously. It also shows that there are no time-locks or multi-signature requirements for these sensitive operations.

Impact

If the owner account is compromised or acts maliciously, they could:

  • Change the Chainlink Functions configuration to point to malicious sources

  • Modify gas limits to cause transactions to fail

  • Update subscription IDs to disrupt the NFT update mechanism

  • Manipulate the system in ways that could lead to loss of functionality or funds

Recommendations

Implement a more decentralized governance approach:

  1. Use a multi-signature wallet or DAO for owner operations

  2. Implement time-locks for sensitive parameter changes

  3. Add emergency pause functionality with community oversight

  4. Consider splitting admin roles using role-based access control (RBAC)

Example implementation using OpenZeppelin's AccessControl:

import "@openzeppelin/contracts/access/AccessControl.sol";
contract WeatherNft is WeatherNftStore, ERC721, FunctionsClient, ConfirmedOwner, AutomationCompatibleInterface, AccessControl {
bytes32 public constant GAS_MANAGER_ROLE = keccak256("GAS_MANAGER_ROLE");
bytes32 public constant CONFIG_MANAGER_ROLE = keccak256("CONFIG_MANAGER_ROLE");
constructor(...) {
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
_setupRole(GAS_MANAGER_ROLE, msg.sender);
_setupRole(CONFIG_MANAGER_ROLE, msg.sender);
}
function updateFunctionsGasLimit(uint32 newGasLimit) external onlyRole(GAS_MANAGER_ROLE) {
s_functionsConfig.gasLimit = newGasLimit;
}
// Other functions with appropriate role restrictions
}
Updates

Appeal created

bube Lead Judge 6 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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