Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Contract - Refund Mechanism can make the trickOrTreat function to fail causing a Denial of Service (DOS) / Stuck Funds

udo

Summary

The contract's refund logic can be blocked if the receiving address (msg.sender) is a contract that cannot accept ETH transfers (i.e., lacks a receive() or fallback() function), potentially leading to stuck transactions and locked funds.

Vulnerability Details

The vulnerability exists in both trickOrTreat() and resolveTrick() functions where refunds are processed:

if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}

Example of a contract that would cause this issue:

contract NonReceivingContract {
SpookyTreats spookyTreats;
constructor(address _spookyTreats) {
spookyTreats = SpookyTreats(_spookyTreats);
}
// No receive() or fallback() function defined
function buyTreat(string memory treatName) external payable {
// Deliberately send more ETH than needed
spookyTreats.trickOrTreat{value: 1 ether}(treatName);
}
}

Impact

The vulnerability can lead to several serious issues:

  1. Denial of Service:

    • Malicious contracts can block their own transactions

    • This could be used to manipulate the NFT market

    • Could prevent legitimate sales from completing

  2. Locked Funds:

    • ETH could become permanently stuck in the contract

    • No mechanism to recover stuck refunds

  3. Failed Transactions:

    • Users lose gas fees on failed transactions

Tools Used

Manual code review

Recommendations

Pull Over Push Pattern

contract SpookySwap {
mapping(address => uint256) public pendingRefunds;
function trickOrTreat(string memory _treatName) public payable {
// ... existing code ...
if (msg.value > requiredCost) {
pendingRefunds[msg.sender] += msg.value - requiredCost;
}
}
+ function withdrawRefund() external {
uint256 refundAmount = pendingRefunds[msg.sender];
require(refundAmount > 0, "No refund available");
pendingRefunds[msg.sender] = 0;
(bool success,) = msg.sender.call{value: refundAmount}("");
require(success, "Withdrawal failed");
}
}
+ mapping(address => uint256) public pendingRefunds;
​
function trickOrTreat(string memory _treatName) public payable {
// ... existing code ...
- if (totalPaid > requiredCost) {
- uint256 refund = totalPaid - requiredCost;
- (bool refundSuccess,) = msg.sender.call{value: refund}("");
- require(refundSuccess, "Refund failed");
- }
+ if (msg.value > requiredCost) {
+ pendingRefunds[msg.sender] += msg.value - requiredCost;
+ }
}
​
​
+ function withdrawRefund() external {
+ uint256 refundAmount = pendingRefunds[msg.sender];
+ require(refundAmount > 0, "No refund available");
+
+ pendingRefunds[msg.sender] = 0;
+ (bool success,) = msg.sender.call{value: refundAmount}("");
+ require(success, "Withdrawal failed");
+}
Updates

Appeal created

bube Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.