First Flight #12: Kitty Connect
First Flight #12: Kitty Connect
Beginner FriendlyFoundryNFTGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Gas-Expensive Constructor in `KittyConnect` Contract lead to Potential DoS Vulnerability

LogeshN

Summary

The constructor in the KittyConnect contract is gas expensive due to a for loop used for initializing an array.

Vulnerability Details

In the KittyConnect contract, the constructor initializes an array of shop addresses using a for loop. However, this approach can be inefficient, especially if the array size is large. Gas consumption in Ethereum is directly proportional to the computational complexity of the contract's execution. Therefore, using a for loop to initialize an array with a large number of elements can result in significant gas costs.

Code Snippet

Relevant code snippet for constructor:

https://github.com/Cyfrin/2024-03-kitty-connect/blob/c0a6f2bb5c853d7a470eb684e1954dba261fb167/src/KittyConnect.sol#L61C1-L70C1

constructor(address[] memory initShops, address router, address link) ERC721("KittyConnect", "KC") {
for (uint256 i = 0; i < initShops.length; i++) {
s_kittyShops.push(initShops[i]);
s_isKittyShop[initShops[i]] = true;
}
​
i_kittyConnectOwner = msg.sender;
i_kittyBridge = new KittyBridge(router, link, msg.sender);
}

Impact

This can be a Medium to High vulnerability. The impact of this vulnerability is significant, as it can lead to a DoS attack.

Proof of Concept

  • Create a new test file called GasLimitConstructor.t.sol.

  • Paste the following code in the test file.

  • Run forge test --match-path test/GasLimitConstructor.t.sol -vvvvv in terminal.

Code
// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.0;
​
import {Test, console} from "lib/forge-std/src/Test.sol";
import {KittyConnect} from "src/KittyConnect.sol";
​
​
contract TestKittyConnect {
// Declaring public variables to hold instances of the KittyConnect contract and addresses of shops, router, and link contracts
KittyConnect public kittyConnect;
address[] private shops;
address public routerAddress;
address public linkAddress;
​
// Function executed before each test case to set up the environment
function beforeEach() public {
// Setting the first element of the array to the address of the current contract
shops[0] = address(this);
​
// Deploying a new instance of the KittyConnect contract with a specified array of shop addresses,
// router address, and link address
kittyConnect = new KittyConnect(shops, address(0x123), address(0x456));
}
​
// Test case to check the gas consumption of deploying the KittyConnect contract
function testConstructorGas() public {
// Getting the gas limit at the start of the function execution
uint gasLimit = gasleft();
​
// Deploying a new instance of the KittyConnect contract within the test function
kittyConnect = new KittyConnect(shops, address(0x123), address(0x456));
​
// Calculating the gas used during the deployment
uint gasUsed = gasLimit - gasleft();
​
// Printing the gas used to the console
console.log("Gas Used:", gasUsed);
​
// Asserting that the gas used is below a certain threshold (400000)
assert(gasUsed < 400000);
}
}
OUTPUT:
Logs:
Gas Used: 4727989

The gas is used more x10 than the limit specified in KittyBridge contract's constructor's gaslimit = 400000.

Tools Used

VSC, manual review, foundry.

Recommended Mitigation Steps

Instead of initializing s_kittyShops and s_isKittyShop directly in the constructor, you can have a factory function which intialize the initializeShops with proper access control onlyKittyConnectOwner modifier that allows adding shop addresses individually.

constructor(address router, address link) ERC721("KittyConnect", "KC") {
i_kittyConnectOwner = msg.sender;
i_kittyBridge = new KittyBridge(router, link, msg.sender);
}
​
function initializeShops(address[] memory initShops) external onlyKittyConnectOwner {
for (uint256 i = 0; i < initShops.length; i++) {
s_isKittyShop[initShops[i]] = true;
s_kittyShops.push(initShops[i]);
}
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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