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
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 {
owner = address(0x1);
attacker = address(0x2);
user = address(0x3);
vm.startPrank(owner);
weatherNft = new WeatherNft(
);
vm.stopPrank();
vm.deal(owner, 10 ether);
vm.deal(attacker, 10 ether);
vm.deal(user, 10 ether);
}
function testCompromisedOwner() public {
vm.startPrank(attacker);
vm.assume(attacker == owner);
string memory maliciousSource = "function weather() { return {weather_enum: 0, temp: 0}; }";
weatherNft.updateSource(maliciousSource);
weatherNft.updateFunctionsGasLimit(1000);
weatherNft.updateKeeperGaslimit(1000);
weatherNft.updateSubId(999999);
bytes memory maliciousSecretsURL = bytes("https://malicious-url.com/secrets");
weatherNft.updateEncryptedSecretsURL(maliciousSecretsURL);
vm.stopPrank();
vm.startPrank(user);
vm.stopPrank();
}
function testNoTimelock() public {
vm.startPrank(owner);
uint32 originalGasLimit = 100000;
uint32 newGasLimit = 50000;
weatherNft.updateFunctionsGasLimit(newGasLimit);
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:
Use a multi-signature wallet or DAO for owner operations
Implement time-locks for sensitive parameter changes
Add emergency pause functionality with community oversight
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;
}
}