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

Absence of shop partner removal functionality in `KittyConnect.sol` impacting on maintenance, security, and user experience

seeu

Summary

The projects, specifically the contract KittyConnect.sol allows the addition of shop partners through the KittyConnect:addShop function. However there is no possibility to remove these entries. The absence of removal functionality poses potential challenges in managing and updating the list of registered shop partners.

Vulnerability Details

The vulnerability arises from the inability to revoke the status of a shop partner once it has been granted. Without the capability to remove a shop partner, the contract's owner cannot effectively update the list of authorized shops. This can lead to an accumulation of inactive or unauthorized shop partners, potentially compromising the integrity and security of the system.

Impact

The impact of this vulnerability could be significant:

  1. Lack of Maintenance: The inability to remove inactive or unauthorized shop partners may result in an outdated list, leading to confusion and inefficiency in managing partnerships;

  2. Security Risks: Inactive, unauthorized or malicious shop partners still have access to certain functions within the contract, posing security risks or creating potential attack vectors. For example, they can still mint NFTs without being restricted;

  3. User Experience: Users may encounter difficulties in distinguishing between active and inactive shop partners, potentially leading to erroneous transactions or interactions.

Tools Used

Manual code review

Recommendations

It is advisable to develop and implement a function to remove shop partners from the contract's list of authorized partners. This function should be accessible only to the contract owner and include proper access controls and validation checks. An example could be the following:

Code
event ShopPartnerRemoved(address partner);
​
/**
* @notice Allows the owner of the protocol to remove a shop partner
* @param shopAddress The address of the shop partner to be removed
*/
function removeShop(address shopAddress) external onlyKittyConnectOwner {
require(s_isKittyShop[shopAddress], "KittyConnect__NotAShopPartner");
​
// Remove the shop partner from the mapping and array
delete s_isKittyShop[shopAddress];
for (uint256 i = 0; i < s_kittyShops.length; i++) {
if (s_kittyShops[i] == shopAddress) {
s_kittyShops[i] = s_kittyShops[s_kittyShops.length - 1];
s_kittyShops.pop();
break;
}
}
​
emit ShopPartnerRemoved(shopAddress);
}

You can also add this test function to your KittyConnect.t.sol test suite to verify that it works correctly

Code
function test_removePartnerShop() public {
address partnerC = makeAddr("partnerC");
address partnerD = makeAddr("partnerD");
address partnerE = makeAddr("partnerE");
​
vm.startPrank(kittyConnectOwner);
kittyConnect.addShop(partnerC);
kittyConnect.addShop(partnerD);
kittyConnect.addShop(partnerE);
vm.stopPrank();
​
assertEq(kittyConnect.getKittyShopAtIdx(2), partnerC);
assertEq(kittyConnect.getKittyShopAtIdx(3), partnerD);
assertEq(kittyConnect.getKittyShopAtIdx(4), partnerE);
​
// You should not be able to remove a shop partner if you are not kittyConnectOwner
vm.startPrank(user);
vm.expectRevert();
kittyConnect.removeShop(partnerD);
vm.stopPrank();
​
// Remove shop partner as kittyConnectOwner
vm.startPrank(kittyConnectOwner);
kittyConnect.removeShop(partnerD);
vm.stopPrank();
​
assert(!kittyConnect.getIsKittyPartnerShop(partnerD));
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
seeu Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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