BoxedL2 Static Analysis Findings

BL2-01S: Illegible Numeric Value Representations

Type	Severity	Location
Code Style		BoxedL2.sol: • I-1: L35 • I-2: L36 • I-3: L189 • I-4: L190

Description:

The linked representations of numeric literals are sub-optimally represented decreasing the legibility of the codebase.

Example:

Copy
35uint256 public constant MAX_SUPPLY = 10000;

Recommendation:

To properly illustrate each value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation:

The referenced value literals have been updated in their representation to 10_000, 100_00, 100_000, and 50_000 respectively in accordance with the recommendation's underscore style, addressing this exhibit.

BL2-02S: Inexistent Event Emissions

Type	Severity	Location
Language Specific		BoxedL2.sol: • I-1: L149-L153 • I-2: L156-L161 • I-3: L164-L169 • I-4: L171-L175 • I-5: L179-L181 • I-6: L183-L185

Description:

The linked functions adjust sensitive contract variables yet do not emit an event for it.

Impact:

149|153|152

Example:

Copy
179function setSubscriptionId(uint256 _subscriptionId) external onlyRole(DEFAULT_ADMIN_ROLE) {
180    subscriptionId = _subscriptionId;
181}

Recommendation:

We advise an event to be declared and correspondingly emitted for each function to ensure off-chain processes can properly react to this system adjustment.

Alleviation:

The TreasurySet, TokensPerBoxSet, FeeBPSSet, StartTimestampSet, SubscriptionIdSet, and KeyHashSet events were introduced to the codebase and are correspondingly emitted in the BoxedL2::setTreasury, BoxedL2::setTokensPerBox, BoxedL2::setFeeBPS, BoxedL2::setStartTimestamp, BoxedL2::setSubscriptionId, and BoxedL2::setKeyHash functions respectively, addressing this exhibit in full.

BL2-03S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		BoxedL2.sol:L52-L80

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
52constructor(
53    string memory _name,
54    string memory _symbol,
55    string memory _standardBaseURI,
56    string memory _goldBaseURI,
57    uint256 _tokensPerBox,
58    uint256 _feeBPS,
59    uint256 _startTimestamp,
60    address _meme,
61    address _unboxed,
62    address _treasury,
63    address _vrfCoordinator
64) 
65    ERC721(_name, _symbol)
66    VRFConsumerBaseV2Plus(_vrfCoordinator)
67{
68    _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
69
70    standardBaseURI = _standardBaseURI;
71    goldBaseURI = _goldBaseURI;
72
73    tokensPerBox = _tokensPerBox;
74    feeBPS = _feeBPS;
75    startTimestamp = _startTimestamp;
76
77    meme = IERC20(_meme);
78    unboxed = IUnboxed(_unboxed);
79    treasury = _treasury;
80}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation:

All input arguments of the BoxedL2::constructor function are adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.

BL2-04S: Multiple Top-Level Declarations

Type	Severity	Location
Code Style		BoxedL2.sol:L13, L17

Description:

The referenced file contains multiple top-level declarations that decrease the legibility of the codebase.

Example:

Copy
13interface IUnboxed {
14    function mint(address to, uint256 tokenId, bool isGold) external;
15}
16
17contract BoxedL2 is ERC721, ERC721Enumerable, AccessControl, ReentrancyGuard, VRFConsumerBaseV2Plus {

Recommendation:

We advise all highlighted top-level declarations to be split into their respective code files, avoiding unnecessary imports as well as increasing the legibility of the codebase.

Alleviation:

The referenced top-level declaration(s) beyond the one that has the same name as the code file have been relocated to their dedicated file(s) (IUnboxed) and are instead imported as advised, increasing the code's clarity.

BL2-05S: Variable Mutability Specifiers (Constant)

Type	Severity	Location
Gas Optimization		BoxedL2.sol: • I-1: L30 • I-2: L49 • I-3: L50

Description:

The linked variables are assigned to only once during their own declaration.

Example:

Copy
30uint256 public mintLimit = 20;

Recommendation:

We advise them to be set as constant greatly optimizing their read-access gas cost.

Alleviation:

The mintLimit, numWords, and requestConfirmations contract-level variables have been set as constant, optimizing the code's gas cost significantly.

BL2-06S: Improper Invocations of EIP-20 transfer / transferFrom

Type	Severity	Location
Standard Conformity		BoxedL2.sol: • I-1: L105 • I-2: L108 • I-3: L134

Description:

The linked statements do not properly validate the returned bool values of the EIP-20 standard transfer & transferFrom functions. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
105meme.transferFrom(msg.sender, address(this), tokensPerBoxAfterFee * _quantity);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

The SafeERC20 library of the OpenZeppelin dependency is now properly imported to the codebase and its SafeERC20::safeTransferFrom & SafeERC20::safeTransfer functions are correctly invoked in place of all potentially unhandled EIP-20 ERC20::transfer & ERC20::transferFrom invocations, addressing this exhibit.