First Flight #12: Kitty Connect

Summary

The onlyOwner() modifier provided by OpenZeppelin's Ownable contract is a widely recognized and trusted method for restricting access to certain functions within your smart contract to the contract's owner only. It's a part of the OpenZeppelin Contracts library, which is known for its security and adherence to best practices in smart contract development.

Vulnerability Details

onlyKittyConnectOwner modifier does not add any additional logic beyond what Ownable provides, it's generally recommended to use onlyOwner() for its simplicity, security, and the benefits of adhering to a standard.

Impact

Implementing custom access control logic introduces the potential for security vulnerabilities, especially if the custom implementation is not thoroughly tested and audited.

Tools Used

Manual review

Recommendations

Upgradability and Maintainability: Choosing standard patterns and well-known libraries like OpenZeppelin can make your contract easier to upgrade and maintain. If you use custom modifiers, document their behavior thoroughly to aid future development and auditing efforts.

Import Ownable.sol from openzeppelin library

import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
}
Add inheritance to the contract

contract KittyConnect is ERC721, Ownable{...}
}
Refactor KittyConnect::addShop() Add inheritance to the contract

/**
* @notice Allows the owner of the protocol to add a new shop partner
* @param shopAddress The address of new shop partner
*/
function addShop(address shopAddress) external onlyOwner {
s_isKittyShop[shopAddress] = true;
s_kittyShops.push(shopAddress);
emit ShopPartnerAdded(shopAddress);
}
}