Platform: Code4rena
Start Date: 08/03/2023
Pot Size: $60,500 USDC
Total HM: 2
Participants: 123
Period: 7 days
Judge: hansfriese
Id: 220
League: ETH
Rank: 108/123
Findings: 1
Award: $29.67
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xSmartContract
Also found by: 0x1f8b, 0x6980, 0xAgro, 0xSolus, 0xhacksmithh, 0xkazim, ABA, BPZ, BowTiedOriole, ChainReview, DadeKuma, DeFiHackLabs, Deathstore, DevABDee, Diana, Dravee, Dug, Englave, Go-Langer, Haipls, IceBear, Inspex, Jeiwan, Kek, Kresh, Madalad, MatricksDeCoder, MyFDsYours, RaymondFam, Rolezn, SAAJ, Sathish9098, Taloner, Udsen, Viktor_Cortess, atharvasama, ayden, brgltd, btk, carlitox477, catellatech, chaduke, codeislight, deadrxsezzz, descharre, erictee, fatherOfBlocks, favelanky, glcanvas, handsomegiraffe, jasonxiale, jekapi, joestakey, lemonr, luxartvinsec, martin, matrix_0wl, minhquanym, mrpathfindr, nadin, oyc_109, parsely, peanuts, pfedprog, rbserver, rokso, saian, santipu_, scokaf, slvDev, tsvetanovv, ubl4nk, ulqiorra, yamapyblack, zaskoh
29.6697 USDC - $29.67
Vulnerability details
Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it. This was breaking the rule of modularity and modular programming: only import what you need Specific imports with curly braces to allow us to apply this rule better.
File: BYTES2.sol
File: NeoTokyoStaker.sol
Manual Analysis
import {contract1 , contract2} from "filename.sol";
import {Owned} from "solmate/auth/Owned.sol"; import {ERC721} from "solmate/tokens/ERC721.sol"; import {LibString} from "solmate/utils/LibString.sol"; import {MerkleProofLib} from "solmate/utils/MerkleProofLib.sol"; import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol"; import {ERC1155, ERC1155TokenReceiver} from "solmate/tokens/ERC1155.sol"; import {toWadUnsafe, toDaysWadUnsafe} from "solmate/utils/SignedWadMath.sol";
Vulnerability details
Unexpected bugs can be reported in recent versions; Risks related to recent releases Risks of complex code generation changes Risks of new language features Risks of known bugs
0.8.19 (2023-02-22) 0.8.18 (2023-02-01) 0.8.17 (2022-09-08) 0.8.16 (2022-08-08)
For reference, see https://github.com/ethereum/solidity/blob/develop/Changelog.md
File: BYTES2.sol
File: NeoTokyoStaker.sol
Manual Analysis
Use a non-legacy and more battle-tested version
Use 0.8.10
Vulnerability details
Pragma statements can be allowed to float when a contract is intended for consumption by other developers, as in the case of contracts in a library or EthPM package. Otherwise, the developer would need to manually update the pragma in order to compile locally.
For reference, see https://swcregistry.io/docs/SWC-103
File: BYTES2.sol
File: NeoTokyoStaker.sol
Manual Analysis
Ethereum Smart Contract Best Practices - Lock pragmas to specific compiler versions.
For reference, see https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/locking-pragmas/
Vulnerability details
We recommend using a header for Solidity code layout and readability
For reference, see https://github.com/transmissions11/headers
All Contracts
/////////////////////////////////////////////////////////////// TESTING 123 ///////////////////////////////////////////////////////////////
Manual Analysis
Vulnerability details
File: Both Contracts On Scope
Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.
For reference, see https://docs.soliditylang.org/en/v0.8.17/style-guide.html
Functions should be grouped according to their visibility and ordered:
-constructor -receive function (if exists) -fallback function (if exists) -external -public -internal -private -within a grouping, place the view and pure functions last
Vulnerability details
NFT thefts have increased recently, so with the addition of hacked NFTs to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.
Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.
Here is the project example; Manifold
Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code
Manual Analysis
Add to Blacklist function and modifier.
Example
modifier nonBlacklistRequired(address extension) { require(!_blacklistedExtensions.contains(extension), "Extension blacklisted"); _; }
Vulnerability details
All Contracts
Description:
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.
For reference, see https://docs.soliditylang.org/en/v0.8.15/natspec-format.html
Manual Analysis
NatSpec comments should be increased in contracts.
Vulnerability details
Use private constant consistently.
File: NeoTokyoStaker.sol
Manual Analysis
Replace constant private with private constant.
Vulnerability details
Imports can be grouped together
File: BYTES2.sol
File: NeoTokyoStaker.sol
Manual Analysis
Consider importing OZ first, then all interfaces, then all utils.
Vulnerability details
In several locations in the code, numbers like 1e12, 1e18, and 0x20 are used. The same goes for values like type(uint256).max It is pretty easy to make a mistake somewhere, also when comparing values.
File: NeoTokyoStaker.sol
Manual Analysis
We recommend defining constants for the above numbers used throughout the code
#0 - c4-judge
2023-03-17T03:15:45Z
hansfriese marked the issue as grade-b