Trick or Treat

First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

Potential denial of Service by withdrawing funds, on `address.transfer (2300 gasLimit)`.

Summary

Usage of address.transfer built-in method might revert if Owner is a contract containing heavy operations on receive or fallback since transfer only supports a max gas usage of gasLimit = 2300 . producing DOS which locks the funds of SpookySwap forever.

Vulnerability Details

built-in transfer function not recommended.

function withdrawFees() public onlyOwner {
uint256 balance = address(this).balance;
@> payable(owner()).transfer(balance);
emit FeeWithdrawn(owner(), balance);
}

Impact

Protocol's Funds stay locked unless recipient handles receive ether under less than 2300 gas. SpookySwap:changeOwner mechanism doesn’t directly mitigate the risk of funds being inaccessible.

Tools Used

  • Manual Review

  • Foundry testing tool

contract Owner {
uint256 val;
receive() external payable {
uint256 gas = gasleft();
uint256 consumed;
//100
while (consumed <= 2300) {
val = type(uint256).max;
consumed += gas - gasleft();
gas = gasleft();
}
}
}
function testDOSonWithdrawFees() public {
Owner owner = new Owner();
SpookySwap.Treat memory treat = SpookySwap.Treat("candy", 0.1 ether, "ipfs://candy-cid");
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
treats[0] = treat;
vm.prank(address(owner));
SpookySwap _protocol = new SpookySwap(treats);
vm.prank(user);
_protocol.trickOrTreat{ value: 0.2 ether }("candy");
vm.prank(address(owner));
vm.expectRevert();
_protocol.withdrawFees();
}

Recommendations

  • use low level address.call instead

Updates

Appeal created

bube Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Use of `transfer` instead of `call`

Support

FAQs

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