Bid Beasts

First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: low
Valid

Missing Event Emission for Failed Transfers

Root + Impact

  • Root Cause: The _payout function handles failed Ether transfers by crediting the amount to failedTransferCredits[recipient] when (bool success, ) = payable(recipient).call{value: amount}("") returns false, but it does not emit an event to log this action. This omission leaves no on-chain record of the failure or the subsequent crediting.

  • Impact: The lack of event emission reduces transparency, making it challenging for users and developers to detect failed transfers or monitor funds credited to failedTransferCredits. This can lead to debugging difficulties, decreased user trust, and potential operational inefficiencies, especially in a marketplace handling significant transactions.

Description:

The BidBeastsNFTMarket::_payout function handles failed Ether transfers by silently crediting the amount to failedTransferCredits[recipient] without emitting an event. This lack of transparency makes it difficult for users or developers to detect and debug failed transfers. Emitting an event when a transfer fails would improve traceability and allow off-chain systems to monitor contract behavior effectively.

function _payout(address recipient, uint256 amount) internal {
if (amount == 0) return;
(bool success, ) = payable(recipient).call{value: amount}("");
if (!success) {
failedTransferCredits[recipient] += amount;
@> // No event emitted
}
}

Risk

  • Likelihood: Medium. The issue affects all failed transfers, a scenario that may occur due to gas limits, recipient contract reverts, or out-of-gas conditions. The likelihood increases with active usage or interactions with untrusted contracts.

  • Impact: Medium. While not causing direct financial loss, the lack of transparency hinders monitoring and debugging, potentially leading to unresolved issues, user frustration, and reduced trust in the platform’s reliability.

Proof of Concept

The following analysis demonstrates the absence of event emission for failed transfers.

Test Case

Add the following to BidBeastsNFTMarketTest.t.sol and run with:

forge test --mt test_NoEventOnFailedTransfer
function test_NoEventOnFailedTransfer() public {
address recipient = makeAddr("Recipient");
uint256 amount = 10 ether;
// Arrange: Fund the contract and simulate a failed transfer (e.g., recipient reverts)
vm.deal(address(market), amount);
vm.etch(recipient, hex"fe"); // Create a contract that reverts all calls
assertEq(market.failedTransferCredits(recipient), 0, "Initial credits should be zero");
// Act: Call _payout (assuming internal call via a public wrapper or cheatcode)
market.payout(recipient, amount); // Assume a public wrapper exists for testing
// Assert: Verify credits are updated but no event is emitted
assertEq(market.failedTransferCredits(recipient), amount, "Credits should be updated");
vm.expectEmit(false, false, false, true);
emit TransferFailedCredited(recipient, amount); // Should not emit
}

Explanation

  • Setup: The contract is funded, and a recipient address is set to revert all calls, simulating a failed transfer.

  • Issue: The _payout function credits 10 ether to failedTransferCredits[recipient] but emits no event, as verified by vm.expectEmit(false, ...).

  • Result: The test confirms the lack of event emission, highlighting the transparency gap for failed transfers.

Recommended Mitigation:

  • Emit an event when a transfer fails to notify users and developers of the failure and the crediting of funds to failedTransferCredits.

  • Use a custom error instead of silent failure to improve clarity and save gas.

  • Example:

+ event TransferFailedCredited(address indexed recipient, uint256 amount);
function _payout(address recipient, uint256 amount) internal {
if (amount == 0) return;
(bool success, ) = payable(recipient).call{value: amount}("");
if (!success) {
failedTransferCredits[recipient] += amount;
+ emit TransferFailedCredited(recipient, amount);
}
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: Incorrect Event Emission

placeBid emits AuctionSettled even though the auction hasn’t ended, causing misleading event logs.

Support

FAQs

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