Skip to content

Latest commit

 

History

History
171 lines (119 loc) · 10.1 KB

solidityinspector_report#kittensonchain.md

File metadata and controls

171 lines (119 loc) · 10.1 KB
Raw

SolidityInspector Analysis Report

This report was generated by SolidityInspector a tool made by Riccardo Malatesta (@seeu). The purpose of this report is to assist in the process of identifying potential security weaknesses and should not be relied upon for any other use.

Table of Contents

Summary

Files analyzed

Filepath SLOC
src/KittensOnChain.sol 110

Issues found

Category Number of issues found
Medium Issues 1
Low Issues 2
Non-Critical Issues 4
Gas Issues 1

Medium Issues

ID Issues Contexts Instances
M-01 Centralization risk detected: contract has a single point of control 1 2

[M-01] Centralization risk detected: contract has a single point of control

Centralization risks are weaknesses that malevolent project creators as well as hostile outside attackers can take advantage of. They may be used in several forms of attacks, including rug pulls. When contracts have a single point of control, contract owners need to be trusted to prevent fraudulent upgrades and money draining since they have privileged access to carry out administrative chores. Some solutions to this issue include implementing timelocks and/or multi signature custody. Reference: Trusting a Smart Contract Means Trusting Its Owners: Understanding Centralization Risk, UK Court Ordered Oasis to Exploit Own Security Flaw to Recover 120k wETH Stolen in Wormhole Hack.

Instances (2)

src/KittensOnChain.sol
::14 => contract KittensOnChain is ERC721, Ownable {
::50 =>     ) ERC721("Kitten", "KTN") Ownable(msg.sender) {

Low Issues

ID Issues Contexts Instances
L-01 Compiler version Pragma is non-specific 1 1
L-02 Timestamp dependency: use of block.timestamp (or now) 1 1

[L-01] Compiler version Pragma is non-specific

For non-library contracts, floating pragmas may be a security risk for application implementations, since a known vulnerable compiler version may accidentally be selected or security tools might fallback to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain. References: Version Pragma | Solidity documents, 4.6 Unspecific compiler version pragma | Consensys Audit of 1inch Liquidity Protocol.

Instances (1)

src/KittensOnChain.sol => pragma solidity ^0.8.18;

[L-02] Timestamp dependency: use of block.timestamp (or now)

The timestamp of a block is provided by the miner who mined the block. As a result, the timestamp is not guaranteed to be accurate or to be the same across different nodes in the network. In particular, an attacker can potentially mine a block with a timestamp that is favorable to them, known as "selective packing". For example, an attacker could mine a block with a timestamp that is slightly in the future, allowing them to bypass a time-based restriction in a smart contract that relies on block.timestamp. This could potentially allow the attacker to execute a malicious action that would otherwise be blocked by the restriction. It is reccomended to, instead, use an alternative timestamp source, such as an oracle, that is not susceptible to manipulation by a miner. References: Timestamp dependence | Solidity Best Practices for Smart Contract Security, What Is Timestamp Dependence?.

Instances (1)

src/KittensOnChain.sol
::129 =>                             Strings.toString(uint256(block.timestamp) % 100),

Non-Critical Issues

ID Issues Contexts Instances
NC-01 require()/revert() statements should have descriptive reason strings 1 1
NC-02 Unnamed return parameters 1 6
NC-03 Usage of abi.encodePacked instead of bytes.concat() for Solidity version >= 0.8.4 1 2
NC-04 public function not used internally could be marked as external 1 8

[NC-01] require()/revert() statements should have descriptive reason strings

To increase overall code clarity and aid in debugging whenever a need is not met, think about adding precise, informative error messages to all require and revert statements. References: Error handling: Assert, Require, Revert and Exceptions, Missing error messages in require statements | Opyn Bull Strategy Contracts Audit.

Instances (1)

src/KittensOnChain.sol
::51 =>         require(

[NC-02] Unnamed return parameters

To increase explicitness and readability, take into account introducing and utilizing named return parameters. Reference: Unnamed return parameters | Opyn Bull Strategy Contracts Audit.

Instances (6)

src/KittensOnChain.sol
::143 =>     function getStateOfToken(uint256 tokenId) public view returns (ColorTrait) {
::150 =>     function getYellowKitten() public view returns (string memory) {
::157 =>     function getRedKitten() public view returns (string memory) {
::164 =>     function getBlueKitten() public view returns (string memory) {
::171 =>     function getGreenKitten() public view returns (string memory) {
::178 =>     function getTokenCounter() public view returns (uint256) {

[NC-03] Usage of abi.encodePacked instead of bytes.concat() for Solidity version >= 0.8.4

From the Solidity version 0.8.4 it was added the possibility to use bytes.concat with variable number of bytes and bytesNN arguments. With a more evocative name, it functions as a restricted abi.encodePacked. References: Solidity 0.8.4 Release Announcement, Remove abi.encodePacked #11593.

Instances (2)

src/KittensOnChain.sol
::121 =>                 abi.encodePacked(
::124 =>                         abi.encodePacked(

[NC-04] public function not used internally could be marked as external

public functions in a smart contract that aren't actually used within the contract itself could be marked as external as they serve no purpose internally.

Instances (8)

src/KittensOnChain.sol
::68 =>     function mintNft() public {
::79 =>     function changeColor(uint256 tokenId) public {
::143 =>     function getStateOfToken(uint256 tokenId) public view returns (ColorTrait) {
::150 =>     function getYellowKitten() public view returns (string memory) {
::157 =>     function getRedKitten() public view returns (string memory) {
::164 =>     function getBlueKitten() public view returns (string memory) {
::171 =>     function getGreenKitten() public view returns (string memory) {
::178 =>     function getTokenCounter() public view returns (uint256) {

Gas Issues

ID Issues Contexts Instances Gas Saved
G-01 Use assembly to check for address(0) 1 1 ~6

[G-01] Use assembly to check for address(0)

By checking for address(0) using assembly language, you can avoid the use of more gas-expensive operations such as calling a smart contract or reading from storage. This can save 6 gas per instance. Reference: Solidity Assembly: Checking if an Address is 0 (Efficiently).

Instances (1)

src/KittensOnChain.sol
::105 =>         if (ownerOf(tokenId) == address(0)) {