Skip to content

Instantly share code, notes, and snippets.

@KenMan79
Forked from yuriy77k/ETH_NFTLootBox_report.md
Created June 28, 2021 03:36
Show Gist options
  • Save KenMan79/c768cc0f81fb8aa338954f8b9eba3949 to your computer and use it in GitHub Desktop.
Save KenMan79/c768cc0f81fb8aa338954f8b9eba3949 to your computer and use it in GitHub Desktop.
NFTLootBox Security Audit Report

NFTLootBox Security Audit Report

1. Summary

NFTLootBox.com smart contract security audit report performed by Callisto Security Audit Department

Logic description by developer:

Users purchase lootboxes using their client seed. the contract doesn't use this seed, but the server (which reads the event emitted) does. Since generating random numbers is very difficult on-chain we calculate winnings server-side. once we've determined the user's winnings, they can redeem those winnings through the redeemBulk function.

2. In scope

Commit hash 56ad73a604226e481e78b2dc634001721203ad66

NFTLootbox.sol lib/Context.sol lib/SafeMath.sol lib/Ownable.sol lib/IERC20.sol lib/IERC1155.sol lib/ReentrancyGuard.sol lib/Address.sol

3. Findings

In total, 3 issues were reported including:

  • 1 low severity issues.

  • 2 notes.

No critical security issues were found.

3.1. Wrong import path

Severity: compiler error (note)

Description

The imported files located in the /lib/ folder, but in the code /lib/ is missing that cause compiler error.

Recommendation

Use import like following:

import "./lib/Context.sol";
import "./lib/SafeMath.sol";
import "./lib/Ownable.sol";
import "./lib/IERC20.sol";
import "./lib/IERC1155.sol";
import "./lib/ReentrancyGuard.sol";

3.2. Zero address checking

Severity: low

Description

There is no zero address checking in functions: setTransferAddress, setAuthAddress, updateLootbox

Recommendation

Add zero address checking:

require( _address != address(0) );

3.3. Gas wasting

Severity: note

Description

The modifier nonReentrant is overused that will cause unnecessary gas usage.

Recommendation

The modifier nonReentrant may be removed from functions setTransferAddress, setAuthAddress, updateLootbox where no risk of re-entrance.

4. Conclusion

The audited smart contract can be deployed. Only low severity issues were found during the audit.

5. Revealing audit reports

https://gist.github.com/MrCrambo/f11c21632ca2b02d92d2e8ede91b6d19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment