Puppy Raffle Code Audit Contest

October 31, 2023
Prepared by: [tri-a-sec LTD] Lead Auditors: - stackangel22

Table of Contents
• Table of Contents
• Protocol Summary
• Disclaimer
• Risk Classification
• Audit Details
– Scope
– Severity Criteria
– Summary of Findings
– Tools Used
• High
• Medium
• Low
• Informational
• Gas
Protocol Summary

The Puppy Raffle protocol enables participants to win a dog NFT, participants would enter as an
individual or a group. The protocol draws a winner, after which winnings are sent to the winner and
fees to the owner of the protocol. It will be noted that contract covered in this audit does not include
periphery base64.sol.
Disclaimer
This audit report does not constitute investment advice or a personal recommendation. It does
not consider, and should not be interpreted as considering or having any bearing on, the potential
economics of a token, token sale or any other product, service or other asset. Any entity should not rely
on this report in any way, including for the purpose of making any decisions to buy or sell any token,
product, service or other asset. The scope of this audit is limited to the code mentioned in Protocol
Summary. The Tri-A-Sec team makes all effort to find as many vulnerabilities in the code in the given
time period, but holds no responsibilities for the findings provided in this document. A security audit
by the team is not an endorsement of the underlying business or product. The audit was time-boxed
and the review of the code was solely on the security aspects of the Solidity implementation of the
contracts.

#Audit Details

Vulnerability Detection: We first scan smart contracts with automatic code analyzers, (slither) and
investigate further findings.
We also manually analyze possible attack scenarios to cross-check the result.
Recommendation: We provide some useful advice based on the result of the audit and our findings.

#Scope
The contract in scope is the PuppyRaffle.sol
Severity Criteria
To standardize the evaluation, i define the following terminology based on https://docs.codehawks.com/hawksauditors/how-to-evaluate-a-finding-severity Methodology [11]:
• Likelihood represents how likely a particular vulnerability is to be uncovered and exploited in the
wild;
• Impact measures the technical loss and business damage of a successful attack;
• Severity demonstrates the overall criticality of the risk.
Likelihood and impact are categorized into three ratings: H, M and L, i.e., high, medium and low
respectively. Severity is determined by likelihood and impact and can be classified into four categories
accordingly, i.e., Critical, High, Medium, Low

In total, we found four potential issues. Besides, we have one recommendation.
• High Risk: 3
• Medium Risk: 0
• Low Risk: 1
• Recommendation: 2

Tools Used
Slither was the only static analysis tools used for this purpose

#Findings:

#High:
Severity: High Status: Just Reported
Re-entrancy in refund() function
Description: In the refund function there is a re-entrancy vulnerability identified. Value is sent to the
player before the player address is passed the address(0) “payable(msg.sender).sendValue(entranceFee);”
, which is used for access control for the function. players[playerIndex] = address(0); require(playerAddress != address(0), “PuppyRaffle: Player already refunded, or is not active”);.
A malicious smart contract can callback into the function to keep withdrawing from it as long as there
is available balance there.
function refund(uint256 playerIndex) public { address playerAddress = players[playerIndex]; require(playerAddress == msg.sender, “PuppyRaffle: Only the player can refund”); require(playerAddress
!= address(0), “PuppyRaffle: Player already refunded, or is not active”);
1 payable(msg.sender).sendValue(entranceFee);
2
3 players[playerIndex] = address(0);
Suggestion: A re-entrancy guard in form of a modifier could be used, or import and inherit from openzepplin Re-entrancy guard contract to stop re-entrancy in the function. https://github.com/OpenZeppelin/openzeppelincontracts/blob/release-v4.5/contracts/security/ReentrancyGuard.sol.
example:
modifier noReentrant() { require(!locked, “No re-entrancy”); locked = true; _; locked = false; } function
refund(uint256 playerIndex) public noReentrant { address playerAddress = players[playerIndex];

require(playerAddress == msg.sender, “PuppyRaffle: Only the player can refund”); require(playerAddress
!= address(0), “PuppyRaffle: Player already refunded, or is not active”);
1 payable(msg.sender).sendValue(entranceFee);
2
3 players[playerIndex] = address(0);
4 }
Also, normal check and control could also be introduced to the function. players[playerIndex] =
address(0); must be set before any external calls are made.

Example:

function refund(uint256 playerIndex) public noReentrant { address playerAddress = players[playerIndex]; require(playerAddress == msg.sender, “PuppyRaffle: Only the player can
refund”); require(playerAddress != address(0), “PuppyRaffle: Player already refunded, or is not active”);
players[playerIndex] = address(0); payable(msg.sender).sendValue(entranceFee); }
Severity: High Status: Just Reported
Re-entrancy in selectWinner() function

Description:

The selectWinner() function has a re-entrancy vulnerability, as a external call is made to another
contract without a re-entrancy guard, makes it open to value loss. (bool success,) = winner.call{value:
prizePool}(““), the winner could be a bad actor with a smart contract which ensures that they can call
back into the Raffle contract if value is sent to it.
Malicious Contract sample:
contract reentrancyAttack{
1 address public victim;
2 uint256 public contractBalance;
3 uint256 public amount;
4 uint256 public counter;
5
6 constructor(address _victim) payable{
7 victim = _victim;
8 amount = msg.value;
9 }
10
11 receive() external payable{
12 counter++;
13 withdrawAttack();
14 }
15
16 function payIn() public returns (bool success){
17 (bool success, bytes memory data) = payable(victim).call{value:
amount}(abi.encodeWithSignature("payIn()"));
18 }
19
20 function withdrawAttack() public{
21 if(counter < 4){
22 payable(victim).call(abi.encodeWithSignature("withdraw()"));
23 }
24 }
25
26 function updateContractBalance() public{
27 contractBalance = address(this).balance;
28 }
}

Suggestion:

As highlighted above it is imperative that a re-entrancy guard is applied to the function to
save guard from this malicious actors. Also access control like a onlyowner modifier from Ownable.sol
could be implemented, to ensure only the admin can call this said function.
modifier noReentrant() { require(!locked, “No re-entrancy”); locked = true; _; locked = false; }
1 function selectWinner() external noReentrant onlyowner{
2 require(block.timestamp >= raffleStartTime + raffleDuration, "
PuppyRaffle: Raffle not over");
3 require(players.length >= 4, "PuppyRaffle: Need at least 4 players"
);
4 uint256 winnerIndex =
5 uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp,
block.difficulty))) % players.length;
6 address winner = players[winnerIndex];
7 uint256 totalAmountCollected = players.length * entranceFee;
8 uint256 prizePool = (totalAmountCollected * 80) / 100;
9 uint256 fee = (totalAmountCollected * 20) / 100;
10 totalFees = totalFees + uint64(fee);
11
12 uint256 tokenId = totalSupply();
13
14 // We use a different RNG calculate from the winnerIndex to
determine rarity
15 uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender,
block.difficulty))) % 100;
16 if (rarity <= COMMON_RARITY) {
17 tokenIdToRarity[tokenId] = COMMON_RARITY;
18 } else if (rarity <= COMMON_RARITY + RARE_RARITY) {
19 tokenIdToRarity[tokenId] = RARE_RARITY;
20 } else {
21 tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
22 }
23
24 delete players;
25 raffleStartTime = block.timestamp;
26 previousWinner = winner;
27 (bool success,) = winner.call{value: prizePool}("");
28 require(success, "PuppyRaffle: Failed to send prize pool to winner"
);
29 _safeMint(winner, tokenId);
30 }

Severity: High Status: Just Reported
Precision loss in selectWinner()function

Description:

The selectWinner() function might deal with integer values, which means fractional token values are not
considered. Fractional values introduced will get rounded up, this affects the integrity of the protocol
as the precise amount calculated would be to the nearest whole number ensuring fund loss due to
rounding up.
Suggestion: To handle fractional token values, you should use a fixed-point arithmetic library like
FixedPoint from OpenZeppelin or SafeMath library. These libraries enable you to work with fractional
numbers in your Solidity contract.

#Medium
N/A

Low

Severity: Low Status: Just Reported
There is concern in the enterRaffle(address[] memory newPlayers) function which takes an address
memory, there seem to no be a limit to the number of addresses the array can accept and process.
While gas costs are reduced for memory operations, you’re still constrained by the gas limit per block.
If the gas cost for your array operations exceeds the gas limit, your contract transaction might fails.
function enterRaffle(address[] memory newPlayers) public payable { require(msg.value == entranceFee

• newPlayers.length, “PuppyRaffle: Must send enough to enter raffle”); for (uint256 i = 0; i < newPlayers.length; i++) { players.push(newPlayers[i]); }
  1 // Check for duplicates
  2 for (uint256 i = 0; i < players.length - 1; i++) {
  3 for (uint256 j = i + 1; j < players.length; j++) {
  stackangel22 9
  Puppy Raffle Code Audit Contest October 31, 2023
  4 require(players[i] != players[j], "PuppyRaffle: Duplicate
  player");
  5 }
  6 }
  7 emit RaffleEnter(newPlayers);
  8 }
  Suggestions: A limit to the lenght and numbers of addresses accepted into the array must be implemented. A bad actor might look to put the contract in a bad state by consuming all the gas with a
  considerable large amount of addresses in the array.

#Informational
Notes: There are informational views on the audited contract. There are calls made to undeclared
and undefined functions, like the totalSupply() function in the selectWinner() function, if it was to be
imported it is not part of the imported smart contracts. uint256 tokenId = totalSupply();
Also, the sendValue() in the refund() function was neither declared nor defined. payable(msg.sender).sendValue(entranceFee).
Lastly the _exists() function in the tokenURI(uint256 tokenId) was also not declared nor defined and
also not inherited from the imported contracts. require(_exists(tokenId), “PuppyRaffle: URI query for
nonexistent token”).
This causes parser error that inhibits the contract from compiling.

#Gas
N/A