Skip to content

Latest commit

 

History

History
238 lines (143 loc) · 7.5 KB

report.md

File metadata and controls

238 lines (143 loc) · 7.5 KB

Aderyn Analysis Report

This report was generated by Aderyn, a static analysis tool built by Cyfrin, a blockchain security company. This report is not a substitute for manual audit or security review. It should not be relied upon for any purpose other than to assist in the identification of potential security vulnerabilities.

Table of Contents

Summary

Files Summary

Key Value
.sol Files 1
Total nSLOC 143

Files Details

Filepath nSLOC
src/PuppyRaffle.sol 143
Total 143

Issue Summary

Category No. of Issues
Critical 0
High 0
Medium 1
Low 3
NC 4

Medium Issues

M-1: Centralization Risk for trusted owners

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

  • Found in src/PuppyRaffle.sol Line: 18

     contract PuppyRaffle is ERC721, Ownable {
  • Found in src/PuppyRaffle.sol Line: 192

         function changeFeeAddress(address newFeeAddress) external onlyOwner {

Low Issues

L-1: abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). Unless there is a compelling reason, abi.encode should be preferred. If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead.

  • Found in src/PuppyRaffle.sol Line: 223

                 abi.encodePacked(
  • Found in src/PuppyRaffle.sol Line: 227

                             abi.encodePacked(

L-2: Solidity pragma should be specific, not wide

Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of pragma solidity ^0.8.0;, use pragma solidity 0.8.0;

  • Found in src/PuppyRaffle.sol Line: 2

     pragma solidity ^0.7.6;

L-3: Conditional storage checks are not consistent

When writing require or if conditionals that check storage values, it is important to be consistent to prevent off-by-one errors. There are instances found where the same storage variable is checked multiple times, but the conditionals are not consistent.

  • Found in src/PuppyRaffle.sol Line: 163

             if (rarity <= COMMON_RARITY) {
  • Found in src/PuppyRaffle.sol Line: 165

             } else if (rarity <= COMMON_RARITY + RARE_RARITY) {

NC Issues

NC-1: Missing checks for address(0) when assigning values to address state variables

Assigning values to address state variables without checking for address(0).

  • Found in src/PuppyRaffle.sol Line: 64

             feeAddress = _feeAddress;
  • Found in src/PuppyRaffle.sol Line: 173

             previousWinner = winner; // e vanity, doesn't matter much.
  • Found in src/PuppyRaffle.sol Line: 193

             feeAddress = newFeeAddress;

NC-2: Functions not used internally could be marked external

  • Found in src/PuppyRaffle.sol Line: 82

         function enterRaffle(address[] memory newPlayers) public payable {
  • Found in src/PuppyRaffle.sol Line: 102

         function refund(uint256 playerIndex) public {
  • Found in src/PuppyRaffle.sol Line: 215

         function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {

NC-3: Constants should be defined and used instead of literals

  • Found in src/PuppyRaffle.sol Line: 90

             for (uint256 i = 0; i < players.length - 1; i++) {
  • Found in src/PuppyRaffle.sol Line: 91

                 for (uint256 j = i + 1; j < players.length; j++) {
  • Found in src/PuppyRaffle.sol Line: 139

             require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
  • Found in src/PuppyRaffle.sol Line: 146

             uint256 prizePool = (totalAmountCollected * 80) / 100;
  • Found in src/PuppyRaffle.sol Line: 148

             uint256 fee = (totalAmountCollected * 20) / 100;
  • Found in src/PuppyRaffle.sol Line: 162

             uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;

NC-4: Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

  • Found in src/PuppyRaffle.sol Line: 54

         event RaffleEnter(address[] newPlayers);
  • Found in src/PuppyRaffle.sol Line: 55

         event RaffleRefunded(address player);
  • Found in src/PuppyRaffle.sol Line: 56

         event FeeAddressChanged(address newFeeAddress);