Four agents audited the code as it is at d11a6b1, each in one area, and a judge reproduced, merged and ranked what they found, then read the code once more itself. Nothing in the code was changed or deployed.

Download the report (Markdown)

5 low2 info

• 1.lowbuy()/buyWithPermit() charge the live stampPrice with no buyer-side bound: a reprice mined before a pending buy pulls the new price from any standing allowancecontracts/src/SkinsStampSale.sol:85

          uint256 stampCost = stampPrice * quantity;

  _buy charges stampPrice * quantity as read from storage at execution time, and the buyer passes only quantity: no maxStampCost, no expected price, no deadline. The owner reprices while the sale is open (AUDIT.md section 11: 'if $STAMP moves a lot, the owner changes it'; web/chain.js exposes setStampPrice in the admin panel), so a routine setStampPrice can land between a buyer seeing a quote and their buy executing.

  Any buyer whose allowance exceeds the quoted cost (a wallet 'approve max', an allowance left over from an earlier larger approval; the project's own test_NotEnoughStampOrAllowance and test_CourierSkinsRulesStillApply use type(uint256).max) is then charged the new price silently, up to their whole $STAMP balance.

  The permit path does not protect a buyer who also has a leftover allowance: the stale-value permit fails, the catch swallows it, and _buy proceeds on the old allowance at the new price (reproduced: test_A2 in the scratch suite, 1000e18 taken against a permit signed for 150e18, nonce not consumed). The web client approves exactly the quoted cost, which limits exposure to users who approved more, but the contract itself offers no bound.

  This is a race between an honest owner action and an unprivileged buyer, not a permission bypass, hence low. Fix that keeps the design: add a uint256 maxStampCost argument to buy and buyWithPermit and revert in _buy when stampCost > maxStampCost; the web client already knows the quoted price it displayed. (Merged from audit_math, audit_economics, audit_permissions and audit_flow, who all reported this at line 85/70.)

  whole finding

  State: sale open, stampPrice = 150e18, alice holds 10_000e18 $STAMP and has called stamp.approve(sale, type(uint256).max).

  Alice sees a quote of 300e18 for 2 skins and submits sale.buy(2).

  The owner's setStampPrice(1000e18) is mined first.

  Expected: the buy reverts or alice pays at most 300e18.

  Actual: sale.buy(2) succeeds, stamp.balanceOf(alice) == 8_000e18 and stamp.balanceOf(treasury) == 2000e18.

  Reproduced in test/scratch/Repro.t.sol test_A_BuyChargesLivePriceAgainstOpenAllowance and test_A2_PermitPathWithOpenApprovalChargesNewPrice; the attached proof fails with 'buyer was charged more than the price they saw: 2000000000000000000000 > 300000000000000000000'.

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity ^0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {CourierSkins} from "src/CourierSkins.sol";
  import {StampToken} from "src/StampToken.sol";
  import {SkinsStampSale} from "src/SkinsStampSale.sol";
  
  /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
  contract OwnersOnlyValidator {
      error UnauthorizedTransfer();
  
      function validateTransfer(address caller, address from, address, uint256) external pure {
          if (caller != from) revert UnauthorizedTransfer();
      }
  }
  
  /// Finding: `buy(quantity)` charges `stampPrice * quantity` as read at execution time and the buyer passes no
  /// bound, so a buyer with a standing allowance above the quoted cost is charged whatever `setStampPrice` set
  /// before their transaction was mined.
  ///
  /// The buy is sent through a raw call to `buy(uint256)` so the test compiles whatever shape the fix takes
  /// (a `maxStampCost` argument on a new `buy`, or a replaced signature). It passes once a buy mined after a
  /// reprice either reverts or charges no more than the buyer saw (300e18 for two skins at 150e18).
  contract ProofNoMaxCostTest is Test {
      CourierSkins skins;
      StampToken stamp;
      SkinsStampSale sale;
  
      address owner = makeAddr("owner");
      address treasury = makeAddr("treasury");
      address minter = makeAddr("minter");
      address alice = makeAddr("alice");
  
      uint256 constant ETH_PRICE = 0.004 ether;
      uint256 constant STAMP_PRICE = 150e18;
  
      function setUp() public {
          skins = new CourierSkins(owner, treasury, ETH_PRICE, bytes32(0), address(new OwnersOnlyValidator()), "");
          vm.prank(owner);
          skins.setSaleOpen(true);
          stamp = new StampToken(owner, address(0), 0);
          vm.prank(owner);
          stamp.setMinter(minter);
          vm.prank(minter);
          stamp.mint(alice, 10_000e18);
          sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, STAMP_PRICE);
          vm.prank(owner);
          sale.setOpen(true);
          vm.deal(address(sale), 0.1 ether);
      }
  
      function test_BuyMinedAfterRepriceNeverChargesMoreThanQuoted() public {
          // Alice saw 150e18 per skin and gave the sale an open approval (a wallet "approve max").
          vm.prank(alice);
          stamp.approve(address(sale), type(uint256).max);
          uint256 quoted = 2 * STAMP_PRICE;
  
          // The owner's reprice lands first.
          vm.prank(owner);
          sale.setStampPrice(1000e18);
  
          vm.prank(alice);
          (bool ok,) = address(sale).call(abi.encodeWithSignature("buy(uint256)", 2));
  
          uint256 paid = 10_000e18 - stamp.balanceOf(alice);
          if (ok) {
              assertLe(paid, quoted, "buyer was charged more than the price they saw");
          } else {
              assertEq(paid, 0, "a refused buy must take no $STAMP");
          }
      }
  }

  whole proof

• 2.lowbuyWithPermit() recomputes the permit value from live stampPrice, so any repricing (up or down) voids the signed permit and the swallowed error surfaces as an unrelated allowance errorcontracts/src/SkinsStampSale.sol:77

          try IERC20Permit(address(stamp)).permit(msg.sender, address(this), stampPrice * quantity, deadline, v, r, s) {} catch {}

  The permit value is not a parameter; it is derived as stampPrice * quantity at execution. The buyer signed over oldPrice * quantity (web/chain.js signs s.skins.stamp.price * n read earlier, with a 600 s deadline). If stampPrice changed between signing and mining, ECDSA recovers a different address, ERC2612InvalidSigner is caught by the blanket catch, no allowance is set, and safeTransferFrom reverts ERC20InsufficientAllowance(sale, 0, newCost).

  This happens even when the price was cut and the signed value would cover the new cost, so a buyer who authorised more than enough is refused, and the signature can never be reused for that price. The blanket catch, meant only for a front-run permit (allowance then already in place), also hides ERC2612ExpiredSignature and the value mismatch, pointing the buyer at an approval problem that does not exist.

  Nothing is lost (atomic revert, nonce not consumed), so this is a robustness and diagnosability defect: every permit-path buy in flight at a repricing fails.

  Fix: take uint256 value as a parameter, pass it to permit, and require stampCost <= value (which also gives the permit path the cost bound missing from buy, see the stampPrice finding); only swallow the permit failure when stamp.allowance(msg.sender, address(this)) >= stampCost afterwards, otherwise re-throw. (Merged from audit_math permit finding, audit_economics catch-all finding and audit_flow case 2.)

  whole finding

  State: stampPrice = 150e18.

  Alice signs a permit for 300e18 (2 skins), deadline now+1h, and submits buyWithPermit(2, deadline, v, r, s).

  The owner's setStampPrice(100e18) is mined first.

  Expected: the buy succeeds at 200e18 (within what alice authorised).

  Actual: permit is called with value 200e18, the recovered signer differs, the error is swallowed, and the call reverts ERC20InsufficientAllowance(sale, 0, 200e18); alice holds 0 skins and stamp.nonces(alice) is still 0.

  Reproduced in test/scratch/Repro.t.sol test_B_PermitVoidedByPriceCut; the 1-wei-raise variant (setStampPrice(151e18), revert ERC20InsufficientAllowance(sale, 0, 151e18) instead of a permit error) in test_B2_PermitForOldPriceSurfacesAsAllowanceError.

  whole reproduction

• 3.lowwithdrawToken() only speaks ERC-20: a Courier Skin (or any ERC-721) a holder sends to the sale is permanently stuckcontracts/src/SkinsStampSale.sol:125

          IERC20(token).safeTransfer(to, amount);

  The function is documented as 'Return tokens sent here by mistake', but it calls the ERC-20 transfer(address,uint256) selector through SafeERC20. CourierSkins has no such function and no fallback, so the call reverts with empty data and SafeERC20 bubbles it.

  The most likely asset to land here by mistake is a Courier Skin itself: the sale is the from of every $STAMP-bought skin, so it appears in holders' transfer histories, CourierSkins.transferFrom (unlike safeTransferFrom) performs no receiver check, and the transfer validator allows any holder to move their own skin (caller == from). _buy only ever transfers the ids it minted in the same call, the sale never approves anyone, and it has no onERC721Received, so no path moves the token again; CourierSkins' validator also blocks any third party from moving it.

  Impact: loss of a 0.004 ETH-priced (and after reveal possibly rare) skin for the sender; owner-side recovery is impossible rather than merely unimplemented.

  Fix: add an owner-only withdrawSkin(uint256 tokenId, address to) calling skins.transferFrom(address(this), to, tokenId) (caller == from, which the registry already allows for the hand-over), or make withdrawToken ERC-721-aware. (Merged from all four specialists.)

  whole finding

  State: bob owns skin #1 (minted for 0.004 ether).

  Bob calls skins.transferFrom(bob, address(sale), 1): succeeds, skins.ownerOf(1) == sale.

  Owner calls sale.withdrawToken(address(skins), bob, 1).

  Expected: skin #1 returns to bob.

  Actual: the call reverts (empty revert data from the missing ERC-20 selector) and skins.ownerOf(1) stays the sale; no function on the sale can move it.

  Reproduced in test/scratch/Repro.t.sol test_C_SkinSentToSaleIsStuck; the attached proof fails with 'no owner function can move a skin out of the sale'.

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity ^0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {CourierSkins} from "src/CourierSkins.sol";
  import {StampToken} from "src/StampToken.sol";
  import {SkinsStampSale} from "src/SkinsStampSale.sol";
  
  /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
  contract OwnersOnlyValidator {
      error UnauthorizedTransfer();
  
      function validateTransfer(address caller, address from, address, uint256) external pure {
          if (caller != from) revert UnauthorizedTransfer();
      }
  }
  
  /// Finding: a Courier Skin a holder moves to the sale with `transferFrom` (allowed: caller == from, and the
  /// unsafe variant has no receiver check) can never leave. `withdrawToken` calls the ERC-20 `transfer` selector,
  /// which CourierSkins lacks, and nothing else in the sale moves an id it did not just mint.
  ///
  /// The recovery is attempted through the plausible owner-only shapes of a fix (an ERC-721-aware
  /// `withdrawToken`, or a new `withdrawSkin` / `withdrawNFT` / `rescueERC721`), so the test passes with any of
  /// them and fails as long as the skin stays on the sale.
  contract ProofStuckSkinTest is Test {
      CourierSkins skins;
      StampToken stamp;
      SkinsStampSale sale;
  
      address owner = makeAddr("owner");
      address treasury = makeAddr("treasury");
      address bob = makeAddr("bob");
  
      uint256 constant ETH_PRICE = 0.004 ether;
  
      function setUp() public {
          skins = new CourierSkins(owner, treasury, ETH_PRICE, bytes32(0), address(new OwnersOnlyValidator()), "");
          vm.prank(owner);
          skins.setSaleOpen(true);
          stamp = new StampToken(owner, address(0), 0);
          sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, 150e18);
          vm.prank(owner);
          sale.setOpen(true);
      }
  
      function _try(bytes memory data) internal returns (bool ok) {
          vm.prank(owner);
          (ok,) = address(sale).call(data);
      }
  
      function test_OwnerCanReturnASkinSentToTheSale() public {
          vm.deal(bob, 1 ether);
          vm.prank(bob);
          skins.mint{value: ETH_PRICE}(1);
          vm.prank(bob);
          skins.transferFrom(bob, address(sale), 1);
          assertEq(skins.ownerOf(1), address(sale));
  
          bool moved = _try(abi.encodeWithSignature("withdrawToken(address,address,uint256)", address(skins), bob, 1))
              || _try(abi.encodeWithSignature("withdrawSkin(uint256,address)", 1, bob))
              || _try(abi.encodeWithSignature("withdrawSkin(address,uint256)", bob, 1))
              || _try(abi.encodeWithSignature("withdrawNFT(address,address,uint256)", address(skins), bob, 1))
              || _try(abi.encodeWithSignature("withdrawERC721(address,address,uint256)", address(skins), bob, 1))
              || _try(abi.encodeWithSignature("rescueERC721(address,address,uint256)", address(skins), bob, 1));
  
          assertTrue(moved, "no owner function can move a skin out of the sale");
          assertEq(skins.ownerOf(1), bob, "the skin is stuck on the sale");
      }
  }

  whole proof

• 4.lowrenounceOwnership is inherited unguarded: one owner call strands the ETH float and freezes price, pause and treasury forevercontracts/src/SkinsStampSale.sol:27

  contract SkinsStampSale is Ownable2Step, ReentrancyGuard {

  Ownable2Step makes transferOwnership two-step but leaves Ownable.renounceOwnership a single irreversible call.

  After it, withdrawEth, setStampPrice, setOpen and setTreasury all revert OwnableUnauthorizedAccount, so the float (0.04 ETH at the launch.env setting, plus any top-ups, which anyone can send through receive()) can only ever leave through purchases, the sale can never be paused or repriced if $STAMP moves, and once Courier Skins' sale is closed for good at reveal (setSaleOpen reverts AlreadyRevealed) any remaining float is locked permanently.

  CourierSkins in the same repository overrides renounceOwnership with a NotFinished guard for exactly this reason, and AUDIT.md describes a launch policy of renouncing every owner, which makes an accidental renounce of the sale plausible; the sale has no guard. Owner-only trigger (self-inflicted), hence low.

  Fix: function renounceOwnership() public override onlyOwner { if (address(this).balance != 0 || open) revert ...; super.renounceOwnership(); }, mirroring CourierSkins, or document that the sale must never be renounced. (Merged from audit_math, audit_permissions and audit_flow.)

  whole finding

  State: sale open and holding 0.1 ETH float.

  Owner calls sale.renounceOwnership().

  Expected: refused while the float is non-zero (as CourierSkins refuses before it is finished).

  Actual: owner() becomes address(0); owner's withdrawEth(owner, 0.1 ether) and setOpen(false) both revert OwnableUnauthorizedAccount(owner), and the 0.1 ETH stays in the contract.

  Reproduced in test/scratch/Repro.t.sol test_D_RenounceStrandsFloat; the attached proof fails with 'renounce went through with 0.1 ether in the float'.

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity ^0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {CourierSkins} from "src/CourierSkins.sol";
  import {StampToken} from "src/StampToken.sol";
  import {SkinsStampSale} from "src/SkinsStampSale.sol";
  
  /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
  contract OwnersOnlyValidator {
      error UnauthorizedTransfer();
  
      function validateTransfer(address caller, address from, address, uint256) external pure {
          if (caller != from) revert UnauthorizedTransfer();
      }
  }
  
  /// Finding: `renounceOwnership` is inherited unguarded from Ownable2Step. One call while the float holds ETH
  /// strands it (only `withdrawEth`, owner-only, takes ETH out other than a purchase) and freezes the price, the
  /// pause switch and the treasury for good. CourierSkins in the same repository guards its renounce; the sale
  /// does not. Passes once a renounce with ETH in the float (or with the sale open) is refused.
  contract ProofRenounceTest is Test {
      CourierSkins skins;
      StampToken stamp;
      SkinsStampSale sale;
  
      address owner = makeAddr("owner");
      address treasury = makeAddr("treasury");
  
      function setUp() public {
          skins = new CourierSkins(owner, treasury, 0.004 ether, bytes32(0), address(new OwnersOnlyValidator()), "");
          stamp = new StampToken(owner, address(0), 0);
          sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, 150e18);
          vm.prank(owner);
          sale.setOpen(true);
          vm.deal(address(sale), 0.1 ether);
      }
  
      function test_RenounceIsRefusedWhileTheFloatHoldsEth() public {
          vm.prank(owner);
          (bool ok,) = address(sale).call(abi.encodeWithSignature("renounceOwnership()"));
          assertFalse(ok, "renounce went through with 0.1 ether in the float");
          assertEq(sale.owner(), owner);
          // The float is still reachable.
          vm.prank(owner);
          sale.withdrawEth(payable(owner), 0.1 ether);
          assertEq(owner.balance, 0.1 ether);
      }
  }

  whole proof

• 5.lowThe only test of the live OpenSea registry reports PASS when run without a fork, and silently stops testing once the Courier Skins sale closescontracts/test/SkinsStampSale.t.sol:290

          if (block.chainid != 4663) return;

  test_Fork_BuyWithStampOnTheLiveContracts is the only test exercising the real StrictAuthorizedTransferSecurityRegistry (0xA000027A9B2802E1ddf7000061001e5c005A0000); the unit suite substitutes OwnersOnlyValidator, a local approximation. AUDIT.md section 11 cites this test as the evidence that the sale-to-buyer hand-over passes OpenSea's registry, one of the brief's explicit questions.

  Off-fork the test returns early and Foundry counts it as passed (gas: 167), so a plain forge test shows 16/16 green whether or not the live registry was ever checked, and the second early return (!SKINS.saleOpen()) turns it off for good once the ETH sale closes, still reporting PASS.

  For the record, this reviewer ran the test with --fork-url https://robinhood.drpc.org and it genuinely passes (gas: 636687): the sale's transferFrom(sale, buyer, id) with caller == from is accepted by the live registry and the buyer can move the skin on.

  Fix: replace the first return with vm.skip(true) (reported as skipped), and assert rather than return on saleOpen so a closed sale is visible; keep a fork run in the pre-deploy checklist. (Merged from all four specialists.)

  whole finding

  Run forge test --match-contract SkinsStampSaleForkTest without --fork-url.

  Expected: the test is reported as skipped.

  Actual on this tree: [PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167) with no assertion executed.

  With --fork-url https://robinhood.drpc.org the same test runs for real and passes (gas: 636687).

• 6.infoUntested edges in test/SkinsStampSale.t.solcontracts/test/SkinsStampSale.t.sol:39

  contract SkinsStampSaleTest is Test {

  Edges the suite never exercises, each with the input that would exercise it. Items marked 'verified' were run in test/scratch/Repro.t.sol and behave correctly; they are listed so the suite can pin them. (1) setStampPrice landing between an approval and a buy with a standing allowance: overcharge, see the stampPrice finding.

  (2) buyWithPermit after setStampPrice, up or down: reverts with an allowance error, see the permit finding. (3) A garbage or expired permit with an existing allowance: approve(sale, 150e18) then buyWithPermit(1, 0, 0, 0, 0) still buys (verified). (4) The 999 cap reached through the sale: totalMinted = 995 then buy(5) reverts CourierSkins.SoldOut with no $STAMP taken, buy(4) then succeeds (verified).

  (5) Re-entry attempted through buyWithPermit, and the refusal reason: test_NoReentry only asserts refused, which is also true for NeedsTopUp or Closed; the caught selector is ReentrancyGuardReentrantCall (verified). (6) withdrawEth(amount > balance) reverts Errors.InsufficientBalance(balance, amount) in Address.sendValue (verified). (7) A contract buyer: works, since the hand-over is plain transferFrom with no receiver hook (verified).

  (8) buyer == treasury: works, $STAMP moves from the buyer to itself (verified). (9) A Courier Skin sent to the sale: stuck, see the withdrawToken finding. (10) Ownable2Step handover of the sale and renounceOwnership: see the renounce finding.

  (Merged from audit_math and audit_economics.)

  whole finding

  Add the listed calls; e.g. for (5) change GreedyTreasury.receive to try sale.buyWithPermit(1, 0, 0, 0, 0) {} catch (bytes memory e) { err = e; } and assert bytes4(err) == ReentrancyGuard.ReentrancyGuardReentrantCall.selector; for (4) mint 995 with ETH, then expect CourierSkins.SoldOut on sale.buy(5) and assert stamp.balanceOf(alice) unchanged.

• 7.infoTrust assumptions: owner powers over the $STAMP price, the float and the treasury, and the independent Courier Skins ETH price (by design, documented for the record)contracts/src/SkinsStampSale.sol:98

      function setStampPrice(uint256 stampPrice_) external onlyOwner {

  Not defects; the powers the brief asks for, stated with their reach, and the brief's questions answered.

  Verified clean: ETH leaves the contract only via skins.mint{value: skins.price() * quantity} at line 89 (to the immutable Courier Skins contract, which forwards it to its treasury) and via withdrawEth (owner only); $STAMP is pulled before the mint and the whole buy is atomic, so no skin is handed over unpaid and no $STAMP is kept without a skin (a failed mint takes no $STAMP: test_CourierSkinsRulesStillApply, and SoldOut verified in the scratch suite); nonReentrant covers both buy entry points, the only mid-flow callback (ETH to the Courier Skins treasury) is refused with ReentrancyGuardReentrantCall, and mints use _mint while the hand-over uses transferFrom, so no receiver hooks run; the validator is called as a view with caller == from, accepted by the live registry (fork test run by this reviewer).

  Trust: (1) setStampPrice is unrelated to the pool price of $STAMP (accepted in AUDIT.md section 11).

  At the launch.env parameters (START_MCAP_USD 3000 for 21M $STAMP, SKINS_STAMP_PRICE 150e18) one skin costs about 0.02 USD in $STAMP against 0.004 ETH, so until the owner reprices or pauses, any buyer takes skins at a small fraction of the ETH price; the ETH only moves to the Courier Skins treasury (the same operator), so the operator's loss is the discount, not the float. setStampPrice(1) gives skins away for 1 wei each.

  (2) The Courier Skins owner (the same wallet at deploy) controls skins.price(): raising it makes the float pay more per skin at the same stampPrice, and setting skins.treasury to the sale would make the float self-replenishing (unlimited $STAMP sales at zero ETH cost). (3) setTreasury redirects all future $STAMP; withdrawEth can empty the float at any time, which makes pending buys revert NeedsTopUp with no loss; setOpen(false) pauses buys only.

  (4) Third-party top-ups through receive() are donations only the owner can take back. (5) A Courier Skins treasury that reverts on receive, or a rule change in OpenSea's registry, blocks every buy (no loss). (Merged from audit_math, audit_economics and audit_flow.)

  whole finding

  State: sale open, float 0.1 ETH, skins.price() 0.004 ETH.

  Owner calls setStampPrice(1); bob approves 10 wei and calls buy(10).

  Expected (by design): 10 skins to bob for 10 wei of $STAMP, 0.04 ETH from the float to the Courier Skins treasury, float 0.06 ETH (reproduced: test/scratch/Repro.t.sol test_T_FloatOnlyLeavesViaMint).

  Any non-owner calling setStampPrice/setOpen/setTreasury/withdrawEth/withdrawToken reverts OwnableUnauthorizedAccount (test_OwnerOnly).

1. Posted15 minto the first attempt
2. Audit economicsAgent #442found 2 low, 3 info

   The review is complete. Findings are in .imd-findings.json at the repository root, with every path, line and snippet verified against the tree.

   Outcome. No critical, high or medium defects in the sale contract. The brief's core questions hold up under test:

   • Float drain. ETH leaves only through the mint call at the skins contract's own price times quantity, or through the owner's withdraw. Nothing else moves it.
   • Skins without paying. The $STAMP pull happens before the mint and the whole buy reverts if any later step fails, so there is no path to an unpaid skin or to kept $STAMP without a skin.
   • Re-entrancy. Both buy entry points are guarded. The only mid-flow callback is the ETH forward to the skins treasury, and it cannot reach an unguarded value-moving function. The validator is a view call, and mints and hand-overs use _mint and transferFrom, so no receiver hooks run.
   • Permit handling. Signer is bound to msg.sender, replay is blocked by the nonce, and a front-run permit falls back to the allowance correctly.
   • Transfer validator. The hand-over is caller equals from, which OpenSea's strict registry allows. I could not verify this on the live chain because the sandbox blocked the Robinhood RPC, so it rests on the repository's own fork test and audit notes.

   Findings written.

   1. Low. buy and buyWithPermit charge the live price with no buyer-side cap. A wallet with a standing max allowance pays whatever setStampPrice set just before its transaction mined. Fix is a maxStampCost argument.
   2. Low. withdrawToken only speaks ERC-20, so a Courier Skin sent to the sale by plain transferFrom is stuck forever. Fix is an owner-only ERC-721 rescue.
   3. Info. The permit try/catch swallows every failure, so a stale-price permit surfaces as a misleading allowance error.
   4. Info. The fork test passes vacuously without a fork, and I listed the untested edges.
   5. Info. Trust assumptions for the owner's powers, documented separately as the brief asks.

   Each finding was confirmed by a scratch test under contracts/test/scratch/, which is not kept. No repository files were changed.

   ran onclaude · claude-fable-5-1 · 27 turns · 9m 37s · 354 in · 30.5K out · 947.7K cached

   submission36e7200e128cd7fcb5f55a04b41e437474dfbaed68b7c4b9029d413bfaa3dcc0

   deviceea89e16822824c6f2a87d26cbd52d3a3bab2b7664b8d92898f6fb5bf24f419ec

   started fromd11a6b102a30e7d130fca4bf45db628f1ae81590

   bundlenone

   • lowbuy()/buyWithPermit() charge the live stampPrice with no buyer-side bound: a standing allowance pays whatever the owner has just setcontracts/src/SkinsStampSale.sol:85

     The $STAMP cost is read from storage at execution time and the buyer has no parameter to cap it (no maxStampCost, no quoted price, no deadline). The owner reprices while the sale is open (README: 'if $STAMP moves a lot, the owner changes it'; the Post Office panel exposes setStampPrice).

     Any buyer whose allowance to the sale exceeds the quoted cost (a wallet 'approve once' of type(uint256).max, the pattern the project's own test_CourierSkinsRulesStillApply uses) and whose buy is mined after a setStampPrice is charged the new price silently, up to their entire $STAMP balance.

     The permit path only protects by accident: a stale permit value makes the permit revert, which is swallowed, and the buy then fails on allowance (see the separate permit finding) unless a prior allowance exists. Amplifier for the admin action: the race between an in-flight buy and setStampPrice, with the buyer as the unprivileged victim; it needs no malice, a routine reprice is enough.

     Fix (keeps the design): add a maxStampCost (or expectedStampPrice) argument to buy/buyWithPermit and revert if stampPrice * quantity exceeds it; the web already knows the quoted price it displayed.

     whole finding

     State: sale open, stampPrice = 150e18, alice holds 10_000e18 $STAMP and has approved the sale for type(uint256).max.

     (1) alice submits buy(1) expecting to pay 150e18.

     (2) owner's setStampPrice(5_000e18) is mined first.

     (3) alice's buy(1) executes.

     Expected: the buy reverts or pays the 150e18 alice saw.

     Actual: stamp.balanceOf(alice) == 5_000e18 and stamp.balanceOf(treasury) == 5_000e18; alice paid 5_000 $STAMP for one skin.

     Confirmed by test_BuyChargesCurrentPriceNotQuotedPrice in test/scratch/SkinsStampSaleAudit.t.sol (passes on current code, i.e. the overcharge happens).

     whole reproduction

   • lowwithdrawToken() only speaks ERC-20: a Courier Skin (or any ERC-721) sent to the sale is permanently stuckcontracts/src/SkinsStampSale.sol:125

     The function is documented as 'Return tokens sent here by mistake', but it calls transfer(address,uint256), a selector ERC-721 contracts do not have. The most likely token to land here by mistake is a Courier Skin itself: the sale is the from of every $STAMP-bought skin, so it appears in holders' transfer histories, and the transfer validator allows any holder to move their own skin to it with plain transferFrom (caller == from).

     There is no ERC-721 rescue path, the sale never approves anyone, and it has no onERC721Received, so the token can never leave.

     Impact: loss of a 0.004 ETH-priced (and post-reveal possibly rare) skin for the sender; owner-only recovery is impossible rather than merely unimplemented.

     Fix: add an owner-only withdrawSkin(uint256 tokenId, address to) that calls skins.transferFrom(address(this), to, tokenId) (allowed by the validator since caller == from), or a generic ERC-721 rescue.

     whole finding

     State: alice owns skin #1 (bought through the sale).

     (1) alice calls skins.transferFrom(alice, address(sale), 1): succeeds, skins.ownerOf(1) == sale.

     (2) owner calls sale.withdrawToken(address(skins), owner, 1).

     Expected: skin #1 returned to owner.

     Actual: the call reverts with empty revert data (OpenZeppelin ERC721 has no transfer(address,uint256) and no fallback; SafeERC20 bubbles the empty revert) and skins.ownerOf(1) stays the sale forever.

     Confirmed by test_SkinSentToSaleIsStuck in test/scratch/SkinsStampSaleAudit.t.sol.

     whole reproduction

   • infobuyWithPermit() swallows every permit failure, so a stale-price or expired permit surfaces as an unrelated allowance errorcontracts/src/SkinsStampSale.sol:77

     The blanket catch is meant for one case, a front-runner having already submitted the permit (its allowance is then in place). It also hides ERC2612ExpiredSignature, ERC2612InvalidSigner and, most commonly, a value mismatch after the owner changed stampPrice between the quote and the mined transaction (the web signs for price * n read earlier).

     Those cases then fail inside safeTransferFrom with ERC20InsufficientAllowance(sale, 0, cost), which points the buyer at an approval problem that does not exist. Security is not affected (nothing is paid and the nonce is not consumed on revert), this is a diagnosability defect.

     Fix: only swallow the permit failure when the allowance already covers the cost, e.g. if (stamp.allowance(msg.sender, address(this)) < cost) { permit(...); } without try/catch, or re-throw the permit error when the post-permit allowance is still below stampCost.

     State: stampPrice = 150e18, alice signs a valid permit(owner=alice, spender=sale, value=150e18, deadline=now+1h).

     Owner calls setStampPrice(151e18) before alice's transaction is mined. alice calls buyWithPermit(1, deadline, v, r, s).

     Expected: a revert naming the permit (value/price mismatch).

     Actual: permit reverts with ERC2612InvalidSigner, the catch discards it, and the transaction reverts with ERC20InsufficientAllowance(sale, 0, 151e18).

     Confirmed by test_PermitForOldPriceSurfacesAsAllowanceError in test/scratch/SkinsStampSaleAudit.t.sol.

     whole reproduction

   • infoThe only test of the real OpenSea registry passes vacuously off-fork, and several sale edges are untestedcontracts/test/SkinsStampSale.t.sol:290

     test_Fork_BuyWithStampOnTheLiveContracts returns early (reported as PASS, gas 167) whenever the suite is not forked, and also returns early forever once SKINS.saleOpen() is false, so a green run of forge test never says anything about the one external behaviour the sale depends on: that OpenSea's StrictAuthorizedTransferSecurityRegistry (0xA000027A9B2802E1ddf7000061001e5c005A0000) accepts the sale's transferFrom(sale, buyer, id) because caller == from.

     The unit suite substitutes OwnersOnlyValidator, a local approximation. This reviewer could not run the fork test (the RPC was blocked in the review sandbox), so that property stands on the repository's own claim in AUDIT.md section 11. Use vm.skip(true) so the result reads 'skipped', and keep a fork run in the pre-deploy checklist.

     Edges the suite does not exercise: a contract buyer (works, plain transferFrom has no receiver hook); an ERC-721 sent to the sale (stuck, see the withdrawToken finding); setStampPrice landing between a quote and a buy with a standing allowance (overcharge, see the stampPrice finding); a permit whose value no longer matches stampPrice (misleading error); withdrawEth for more than the balance (reverts in Address.sendValue); buyer == treasury; Ownable2Step handover of the sale.

     whole finding

     Run forge test --match-path test/SkinsStampSale.t.sol with no --fork-url.

     Expected: the live-registry test is reported as skipped.

     Actual output on this tree: [PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167) while no assertion in it ran.

   • infoTrust assumptions: owner powers over price, float and treasury (by design, documented for the record)contracts/src/SkinsStampSale.sol:98

     Not defects; the powers the brief asks for, stated with their reach. (1) setStampPrice(1) sells skins for 1 wei of $STAMP each, so the float flows to the Courier Skins treasury at skins.price() per skin until it is dry (10 skins per 0.04 ETH top-up at 0.004 ETH): the owner can give skins away, never take a buyer's $STAMP beyond the standing-allowance race above. (2) setTreasury redirects all future $STAMP; past payments are untouched.

     (3) withdrawEth can empty the float at any time, which makes pending buys revert with NeedsTopUp (no loss). (4) setOpen(false) pauses buys only; the owner can still withdraw. (5) The float can be topped up by anyone through receive() and only the owner can take it back, so third-party top-ups are donations.

     (6) The Courier Skins owner (the same wallet at deploy) controls skins.price() and skins.treasury(); setting skins.treasury to the sale would make the float self-replenishing, i.e. unlimited $STAMP sales at zero ETH cost. (7) The transfer validator is OpenSea's registry; a rule change there blocks the sale's hand-over and every buy reverts (no loss).

     Verified clean under the brief's questions: the float only leaves via skins.mint{value: skins.price()*quantity} or withdrawEth; $STAMP is pulled before the mint and the whole buy reverts if any step fails, so no skin is handed over unpaid and no $STAMP is kept without a skin; nonReentrant covers both buy entry points and the only mid-flow callback (ETH to the skins treasury) cannot reach an unguarded value-moving function; the validator is called as a view; mints use _mint and hand-over uses transferFrom, so no receiver hooks run.

     whole finding

     State: sale open, float 0.04 ETH, skins.price() 0.004 ETH.

     Owner calls setStampPrice(1); bob approves 10 wei and calls buy(10).

     Expected (by design): 10 skins to bob for 10 wei of $STAMP, 0.04 ETH from the float to the skins treasury, float 0.

     Any non-owner calling setStampPrice/setOpen/setTreasury/withdrawEth/withdrawToken reverts with OwnableUnauthorizedAccount (covered by test_OwnerOnly).

3. Audit permissionsAgent #629found 2 low, 2 info

   The review is complete and the findings file is written. The tree is otherwise unchanged (the scratch test was removed).

   Outcome. SkinsStampSale has no critical, high or medium defects in the areas the brief asked about. I recorded two low and two informational findings in .imd-findings.json, each with a reproduced failing state.

   What held up under review

   • Float drain. ETH leaves only through skins.mint at exactly price * quantity for a purchase that already pulled the buyer's STAMP, or through the owner's withdrawEth. No overflow, sentinel or reentrant path pays more. That ETH lands in the Courier Skins treasury, which the deploy script makes the same wallet as the sale owner, so the float circulates rather than being lost.
   • Skins without paying. STAMP is pulled before the mint and the whole buy is atomic, so a failed mint, closed sale, sell-out or short float takes nothing. The buyer is always the caller, so a permit or allowance can never be spent for someone else.
   • Permit handling. The permit is wrapped in try/catch, so a front-run permit falls back to the allowance it created. Owner, spender and nonce are bound by the token's EIP-2612 implementation. The signed value is tied to the price at signing time.
   • Reentrancy. Both entry points are guarded. The only callback is the Courier Skins treasury receiving ETH during mint, after the ids are already assigned, so the sale's first..first+quantity-1 ids are correct even if the treasury mints in the middle. The buyer gets transferFrom, so there is no receiver hook.
   • OpenSea validator. I called the live registry on Robinhood Chain with the collection as sender. A caller equal to from passes and any other caller reverts, which matches the sale's self-transfer design.
   • Access control. Every setter and withdrawal is owner-only, ownership is two-step, skins and stamp are immutable, and there is no initializer. The only unguarded state change is the open receive, which is intended.

   Findings recorded

   1. Low. No cost bound on buy. The permit path refuses a buy if the price changed after signing, but buy and a permit buy backed by a leftover open approval charge whatever the price is at execution. A max-cost argument would preserve the owner's pricing right.
   2. Low. Unguarded renounce. The sale inherits a plain renounceOwnership, unlike Courier Skins, in a project whose launch policy renounces everything. Renouncing with a funded float, then the skins sale closing at reveal, locks the ETH forever.
   3. Info. Stuck skins. A holder can move a skin into the sale, and no function can move it out, since withdrawToken only speaks ERC20.
   4. Info. Fork test counts as a pass when not forked. The only test of the real validator returns early instead of skipping, so a green suite does not show the OpenSea path ran.

   Trust assumptions to note, not defects. The owner sets the STAMP price with no link to the pool price, so if STAMP falls in value against ETH, real purchases hand out skins cheaply. The Courier Skins owner can raise the ETH price and drain the float faster per skin. Both roles are the same wallet by the deploy script.

   Untested edges the suite does not cover: a buyer that is a contract, the sell-out path through the sale, a price change between approval and buy, and renouncing the sale.

   ran onclaude · claude-fable-5-1 · 21 turns · 7m 23s · 258 in · 27.5K out · 620.9K cached

   submission0d2558af4a9032b83577e8061cfc9fd47d36f1c03f9a7f2ffea128b3dcbc0294

   devicef9cb4fd544aa3c686146f6a5cd2d7c0fc4d64bd16839e218b8cb752ea0ba94f7

   started fromd11a6b102a30e7d130fca4bf45db628f1ae81590

   bundlenone

   • lowbuy() has no cost bound: with an open $STAMP approval a stampPrice change mined first charges the new price, unlike the permit path which binds the signed valuecontracts/src/SkinsStampSale.sol:70

     Asymmetry between the two paired entry points. buyWithPermit signs stampPrice * quantity, so if the owner changes stampPrice between signing and inclusion the permit fails and (absent another allowance) the buy reverts: the buyer is never charged a price they did not agree to. buy() takes only quantity; _buy charges whatever stampPrice is at execution (line 85).

     A buyer who set an unlimited approval (the pattern the frontend or a wallet 'approve max' prompt produces; the project's own test uses type(uint256).max for the poor buyer) is charged the new price with no way to cap it. The same applies to buyWithPermit when a leftover open approval exists: the permit fails on the value mismatch, the catch swallows it, and the buy proceeds on the old allowance at the new price.

     This needs the trusted owner to move the price, so it is low: it is a missing user-side bound, not a permission bypass.

     Fix: add a maxStampCost (or expectedStampPrice) argument to buy() and buyWithPermit() and revert in _buy if stampPrice * quantity exceeds it; this preserves the owner's right to set the price.

     whole finding

     State: sale open, stampPrice = 150e18, alice holds 10,000e18 STAMP and has called stamp.approve(sale, type(uint256).max).

     Owner's setStampPrice(1000e18) is mined before alice's buy(1).

     Expected (from the buyer's view and from the permit path's behaviour): buy reverts or charges 150e18.

     Actual: buy(1) succeeds, treasury receives 1000e18 and alice's balance is 9,000e18.

     Reproduced in a scratch Foundry test (test_BuyHasNoMaxCost); the permit variant with no other allowance reverts (test_PermitPathBindsCost), and with a leftover max approval buyWithPermit also charges 1000e18 (test_PermitPathWithOpenApprovalChargesNewPrice).

     whole reproduction

   • lowrenounceOwnership is not guarded: renouncing while the float holds ETH strands it, and after Courier Skins' sale ends it can never be spent eithercontracts/src/SkinsStampSale.sol:27

     Ownable2Step's renounceOwnership() is inherited unchanged. The only way ETH leaves the contract other than a purchase is withdrawEth (onlyOwner, line 116), and the only way to stop purchases, change the price or redirect $STAMP is also owner-only.

     This project's launch policy renounces every owner after launch (AUDIT.md: 'After it, nothing in the system has an owner') and CourierSkins.renounceOwnership is explicitly guarded (NotFinished until revealed and frozen) for exactly this reason, but the sale has no such guard.

     If the owner renounces with the float funded, the ETH is reachable only through buys; once Courier Skins' sale closes (which is permanent at reveal, since setSaleOpen reverts AlreadyRevealed), the remaining float is locked forever, stampPrice can never be updated, and the sale can never be closed.

     Fix: override renounceOwnership to revert while address(this).balance != 0 (or while open), mirroring CourierSkins, or document that the sale must never be renounced.

     whole finding

     State: sale owned by owner, float 0.04 ether, Courier Skins sale open. owner calls sale.renounceOwnership().

     Then owner.withdrawEth(owner, 0.04 ether) reverts OwnableUnauthorizedAccount(owner).

     After skins.setSaleOpen(false) (and later reveal, which makes the closure permanent), alice's sale.buy(1) reverts CourierSkins.SaleClosed, so the 0.04 ether can never move.

     Reproduced in a scratch Foundry test (test_RenounceStrandsFloat).

   • infoA Courier Skin transferred to the sale by its holder is unrecoverable: withdrawToken only handles ERC20scontracts/src/SkinsStampSale.sol:122

     The rescue path is asymmetric: stray ERC20s can be returned, stray ERC721s cannot. A holder can move a skin into the sale with transferFrom (the OpenSea registry allows an owner moving its own token, and the sale has no onERC721Received so only the unsafe variant lands).

     Nothing in the sale calls transferFrom on an id it did not just mint (_buy only moves first..first+quantity-1), and withdrawToken calls ERC20 transfer(address,uint256), which CourierSkins does not implement, so the token is stuck. Low impact (user error, cosmetic asset); fix is a withdrawNFT(address token, address to, uint256 id) owner function, or accept and document it.

     State: alice owns skin #1 (minted for 0.004 ether). alice calls skins.transferFrom(alice, sale, 1); ownerOf(1) == sale. owner calls sale.withdrawToken(address(skins), owner, 1): reverts (SafeERC20 call to a selector CourierSkins lacks). ownerOf(1) remains the sale; no function can move it. Reproduced in a scratch Foundry test (test_SkinSentToSaleIsStuck).

   • infoThe fork test against the live OpenSea validator reports PASS when not forked, so the suite never shows whether the validator path rancontracts/test/SkinsStampSale.t.sol:290

     test_Fork_BuyWithStampOnTheLiveContracts is the only test exercising the real StrictAuthorizedTransferSecurityRegistry at 0xA000027A9B2802E1ddf7000061001e5c005A0000; the local tests use a stand-in validator. Without --fork-url it returns early and is counted as a pass (gas 167 in a plain forge test run), so a green suite does not tell the reader that the OpenSea transfer path was checked. Use vm.skip(true) (reported as skipped) or assert the chain id.

     For the record: an eth_call on Robinhood Chain to the live registry with caller == from == 0x1111...1111 and the collection as msg.sender succeeds, and with caller != from reverts StrictAuthorizedTransferSecurityRegistry__UnauthorizedTransfer, confirming the sale's self-transfer design passes the validator as deployed today.

     Run forge test --match-path test/SkinsStampSale.t.sol without a fork.

     Expected: the fork test is reported skipped.

     Actual: [PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167) with no assertion executed.

4. Audit flowAgent #371found 3 low, 2 info

   Review complete. Findings are written to .imd-findings.json at the repository root, with every snippet checked against the tree. No contract files were changed; only scratch tests under contracts/test/scratch/ were added.

   Outcome: no critical or high defects. The brief's five questions all hold on the code as it stands:

   • Float drain. ETH leaves only through the immutable Courier Skins mint at its current price, or through the owner's withdrawEth. No other path exists.
   • Skins without paying. $STAMP is pulled before the mint, the whole buy is atomic, and a failed mint takes nothing.
   • Permit handling. The try/catch makes a front-run permit harmless, and the permit is bound to msg.sender, so nobody can spend another signer's permit.
   • Re-entrancy. Both buy paths are guarded. A re-entering Skins treasury is refused, and the plain transferFrom to the buyer fires no receiver hook.
   • OpenSea validator. I ran the project's fork test plus a contract-buyer variant against the live Robinhood Chain contracts, and both pass. The registry reports the collection at the owner-moves-own level and rejects any other level, so the sale's own-token transfer cannot be configured away.

   Reported findings, all low or informational:

   1. No price bound on buys. A setStampPrice landing ahead of a buy overcharges a buyer with a standing unlimited approval, and voids a permit signed for the old higher price because the permit value is recomputed from storage. Verified in scratch tests.
   2. Stray skin is stuck. A skin sent to the sale with transferFrom cannot be recovered. The ERC-20-only withdrawToken reverts against the ERC-721, and nothing else moves it.
   3. Fork test is green when unforked. The only test touching the live validator returns early off-fork and reports PASS, so a green suite says nothing about the registry. The unit suite also lacks a contract-buyer and a price-change-in-flight case.
   4. Ungated one-step renounce. renounceOwnership works immediately and strands the float and the open state, unlike the gated version in CourierSkins.
   5. Trust assumptions. The Skins owner's ETH price changes and this owner's $STAMP price set how fast the float converts, and the owner can withdraw it all. Documented as the operator's actor list, not defects.

   Coverage note: I applied the execution-trace, periphery, first-principles, access-control, invariant and asymmetry passes to the sale, its interface to CourierSkins and StampToken, the deploy script, and the tests. The vendored OpenZeppelin is 5.4.0. Static analyzers were not run, since none is provided on this machine.

   ran onclaude · claude-fable-5-1 · 30 turns · 7m 45s · 386 in · 31.1K out · 1.1M cached

   submission5185c16b162edf5a53ae3e562ee9d0ac83e6aff79c11a1f79a7eb989e4e47d1e

   device2dc755dfe7bd177cad32d48075604a2bb9fc500add43a0ab0bbcfb24e7f73a55

   started fromd11a6b102a30e7d130fca4bf45db628f1ae81590

   bundlenone

   • lowbuy/buyWithPermit have no price bound: a stampPrice change in flight overcharges standing approvals or voids a sufficient permitcontracts/src/SkinsStampSale.sol:85

     The buyer never states the price they accept. _buy charges whatever stampPrice is at inclusion time, and buyWithPermit recomputes the permit value from the live stampPrice (line 77) rather than taking the signed value.

     Two consequences when the owner's setStampPrice lands between the buyer seeing a price and their transaction executing (Robinhood Chain sequences in order, but a setStampPrice sent moments earlier is still included first): (1) a buyer with a standing unlimited approval, the usual wallet flow, is charged the new higher price with no revert; (2) a buyer who signed a permit for the old, higher price gets a permit that no longer matches, it is swallowed by the try/catch, and the buy reverts with ERC20InsufficientAllowance although the signer authorised more than enough.

     The owner is trusted, so this is a slippage/robustness gap rather than a bypass, but the contract offers no way for the buyer to bound it. Fix that keeps the design: add a maxStampCost (or expectedStampPrice) argument to buy and buyWithPermit and revert when stampCost exceeds it; in buyWithPermit pass the signed value through as a parameter (and still check it covers stampCost) instead of deriving it from storage.

     whole finding

     Verified with contracts/test/scratch/SkinsStampSaleAudit.t.sol (tests test_A1/test_A2).

     Setup as in the project tests (ETH price 0.004 ether, stampPrice 150e18, alice holds 10_000e18 STAMP).

     Case 1: alice stamp.approve(sale, type(uint256).max); owner sale.setStampPrice(1000e18); alice sale.buy(1).

     Expected (what alice saw): 150e18 charged or a revert.

     Actual: succeeds and alice's balance is 9_000e18, 1000e18 taken.

     Case 2: alice signs a permit for value 150e18 (current price); owner sale.setStampPrice(100e18); alice sale.buyWithPermit(1, deadline, v, r, s).

     Expected: the buy goes through (she authorised 150, cost is 100).

     Actual: revert ERC20InsufficientAllowance(sale, 0, 100e18) because permit() was called with value 100e18, which does not match the signature.

     whole reproduction

   • lowA Courier Skin sent to the sale with transferFrom is unrecoverable: withdrawToken only speaks ERC-20 and no path moves an ERC-721 it did not just mintcontracts/src/SkinsStampSale.sol:125

     The NatSpec says the sale never keeps a skin and that the owner can return tokens sent by mistake. CourierSkins.transferFrom (unlike safeTransferFrom) performs no receiver check and the validator allows an owner moving their own token, so any holder can land a skin on the sale address by mistake.

     The only recovery function, withdrawToken, calls transfer(address,uint256) through SafeERC20; CourierSkins has no such function and no fallback, so the call reverts, and _buy only ever transfers the ids it minted in the same call. The skin is stuck forever, with its royalty-bearing value lost to the holder.

     Fix: add an owner-only withdrawSkin(uint256 tokenId, address to) that calls skins.transferFrom(address(this), to, tokenId) (the same owner-moves-own path the validator already accepts), or generalise withdrawToken to accept an ERC-721.

     Verified with contracts/test/scratch/SkinsStampSaleAudit.t.sol (test_B). bob mints skin #1 with ETH, then calls skins.transferFrom(bob, address(sale), 1); it succeeds and skins.ownerOf(1) == address(sale).

     Owner calls sale.withdrawToken(address(skins), owner, 1).

     Expected: the skin is returned.

     Actual: the call reverts (empty revert data bubbled from CourierSkins, which has no transfer(address,uint256)); skins.ownerOf(1) is still the sale and no other function can move it.

   • lowFork test against the live OpenSea registry reports PASS when run without a fork, so a green suite proves nothing about the validatorcontracts/test/SkinsStampSale.t.sol:290

     test_Fork_BuyWithStampOnTheLiveContracts is the only test that exercises the real transfer validator (0xA000027A9B2802E1ddf7000061001e5c005A0000); the unit tests use OwnersOnlyValidator, a stand-in. Off-fork the test returns early and Foundry counts it as passed (gas: 167), so forge test shows 16/16 green whether or not the live registry was ever checked, and the same early return (!SKINS.saleOpen()) silently turns it off once the ETH sale closes.

     The unit suite also never tests a contract buyer (the transferFrom path with no receiver hook) or a stampPrice change between approval and buy.

     For the record, I ran the fork test and an extra contract-buyer variant against Robinhood Chain and both pass: the registry's policy for the collection is level 3 (owner moving own token allowed) and it rejects any other level (StrictAuthorizedTransferSecurityRegistry__UnsupportedSecurityLevelDetail), so the sale's own-token transfer cannot be configured away.

     Fix: use vm.skip(true) (reported as skipped) instead of return, assert rather than return on saleOpen so a closed sale is visible, and add unit tests for a contract buyer and for a price change in flight.

     whole finding

     Run forge test --match-contract SkinsStampSaleForkTest with no --fork-url.

     Expected: the test is reported as skipped (or fails for lack of a fork).

     Actual: [PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167) with nothing executed.

     With --fork-url https://robinhood.drpc.org the same test runs for real (gas: 636687) and passes.

   • inforenounceOwnership is single-step and ungated: one wrong call strands the float and leaves the sale permanently opencontracts/src/SkinsStampSale.sol:27

     Ownable2Step only protects transferOwnership; renounceOwnership is inherited unchanged and takes effect immediately. After it, withdrawEth, setOpen, setStampPrice and setTreasury are all unreachable: the float can leave only through purchases and the sale cannot be paused or repriced. CourierSkins in the same repository gates its renounceOwnership (NotFinished); this contract does not.

     Trust note rather than a bypass.

     Fix: override renounceOwnership to revert, or to require address(this).balance == 0 && !open.

     Verified with contracts/test/scratch/SkinsStampSaleAudit.t.sol (test_D).

     Owner calls sale.renounceOwnership() while the sale holds 0.1 ether and is open.

     Expected: refused, or at least only with an empty, closed sale.

     Actual: owner() becomes address(0); withdrawEth(owner, 0.1 ether) and setOpen(false) revert OwnableUnauthorizedAccount for every caller; the 0.1 ether float is recoverable only by someone buying skins.

   • infoTrust assumptions: the float is spent at whatever ETH price the Courier Skins owner sets, against a $STAMP price only this owner movescontracts/src/SkinsStampSale.sol:83

     Documenting the privileged powers the brief asked about, with the float-drain question answered. ETH leaves the contract only via skins.mint at line 89 (to the immutable Courier Skins contract, which forwards it to its treasury) and via withdrawEth (owner only, line 118).

     $STAMP is pulled before the mint and the whole buy is atomic and nonReentrant, so no buyer gets a skin without paying and no partial state survives a revert; a re-entering Skins treasury is refused (covered by test_NoReentry).

     What remains is trust: (a) the Courier Skins owner (today the same wallet) can raise price, and the sale will keep selling at the fixed stampPrice, spending the float faster per skin; (b) the sale owner sets stampPrice with no relation to the pool price of $STAMP, so if 150 $STAMP trades below 0.004 ETH the float is converted to skins at a discount by ordinary buyers (the ETH still reaches the Skins treasury, so the operator's net loss is the discount, not the float); (c) the owner can withdraw the entire float at any time.

     None of these is a defect against the stated design; they are the actor/precondition list for the operator.

     Optional hardening: have setStampPrice take effect from the next block, or add a maximum ETH price per skin the sale will pay.

     whole finding

     State: sale funded with 0.1 ether, stampPrice 150e18.

     Skins owner calls skins.setPrice(0.01 ether) (allowed, any non-zero value). alice buy(10) then pulls 0.1 ether from the float (the entire float, 25 skins' worth at the old price) for 1_500e18 $STAMP; verified by the existing test_FollowsCourierSkinsPrice pattern.

     Expected per the brief: float leaves only for real purchases, which holds; actual: it does, but at a rate the sale does not cap.

5. Audit mathAgent #184found 4 low, 3 info

   The review is complete. The findings file at the repository root holds 7 entries, no source file was changed, and the scratch tests that back the findings live under contracts/test/scratch/.

   Outcome: no critical, high or medium defect. The float can only leave through CourierSkins.mint after the buyer's $STAMP has already been pulled, every failure path reverts atomically, both entry points are nonReentrant, the permit is correctly treated as optional, and token ids are read immediately before the mint with no external code in between. I verified the hand-over against the live registry: an eth_call to the pinned validator from the Courier Skins address succeeds when caller equals from and reverts for a third-party caller, so the sale's self-transfer passes OpenSea's rules.

   Four low findings, each reproduced in a Foundry test:

   • No buyer-side price cap in buy. A buyer with a standing allowance above the quote pays the post-reprice amount if the owner's setStampPrice lands first. The test shows 2000e18 leaving a wallet quoted 300e18. The web client approves the exact cost, which limits this to unlimited approvals.
   • buyWithPermit derives the permit value from live state. Any repricing, even a price cut, makes the signature mismatch; the error is swallowed and the buy reverts with insufficient allowance. Taking value as a parameter fixes this and the cap gap together.
   • A skin sent to the sale is stuck. The rescue function only speaks ERC-20, and the sale never approves anyone, so an owner-only ERC-721 rescue is needed.
   • renounceOwnership is unguarded. One owner call strands the float and freezes pause and price, while the sibling CourierSkins contract guards against exactly this.

   Three info items: the fork test returns rather than skips so it reports PASS without running, a list of untested edges with the inputs that exercise them, and the documented trust assumption that the $STAMP and ETH prices are independent, with launch numbers showing the gap.

   Coverage: all twelve review passes were applied to the one contract in scope, with the math, boundary and numerical-gap lenses first. There is no division in the contract, so rounding and zero-truncation do not arise; the two multiplications are checked arithmetic that revert. The scratch test file has one intentionally failing case for the overcharge and five passing cases that pin the other behaviours.

   ran onclaude · claude-fable-5-1 · 27 turns · 10m 2s · 418 in · 37.3K out · 1.2M cached

   submission767c9cab66700d947d964c243c8031eae81bb9fdb9284418f55323b22f21d209

   devicefa5c50e7abe465711f0b5c1f6f04d8bd9cb2dbaa6ea0ed86b2e3691a6d7563c5

   started fromd11a6b102a30e7d130fca4bf45db628f1ae81590

   bundlenone

   • lowbuy() has no buyer-side cap on the $STAMP charged: an owner repricing that lands before a pending buy pulls the new price from any standing allowancecontracts/src/SkinsStampSale.sol:85

     _buy charges stampPrice * quantity as read at execution time; the buyer passes only quantity, so nothing in the call encodes the price they agreed to. AUDIT.md states the owner will change stampPrice when $STAMP moves. Any buyer whose allowance to the sale exceeds the quoted cost (the wallet 'unlimited' approval that most wallet UIs offer, or an allowance left from an earlier larger approval) is charged the post-reprice amount if their buy is mined after setStampPrice.

     The web client approves exactly the quoted cost, which limits exposure to users who approved more, but the contract itself offers no bound. This is a race between an honest owner action and an unprivileged buyer, not owner malice.

     Fix: add uint256 maxStampCost to buy (and buyWithPermit, see the permit finding) and revert if stampCost > maxStampCost; this preserves the owner's right to reprice while in-flight buyers only ever pay what they saw.

     whole finding

     State: stampPrice = 150e18, alice has 10_000e18 STAMP and has called stamp.approve(sale, type(uint256).max).

     Alice sees a quote of 300e18 for 2 skins and submits sale.buy(2).

     Owner's setStampPrice(1_000e18) is mined first.

     Expected: alice pays at most 300e18 or the buy reverts.

     Actual: sale.buy(2) succeeds and 2_000e18 STAMP leaves alice (test test_F1_RepriceBeforeBuyPullsMoreThanQuoted in test/scratch/SkinsStampSaleAudit.t.sol fails with 'buyer paid more than quoted: 2000000000000000000000 > 300000000000000000000').

     whole reproduction

   • lowbuyWithPermit recomputes the permit value from live stampPrice, so any repricing (up or down) makes the signed permit unusable and the buy revertscontracts/src/SkinsStampSale.sol:77

     The permit value is not a parameter; it is derived as stampPrice * quantity at execution. The buyer signed over oldPrice * quantity. If stampPrice changed between signing and mining (the deadline the web client uses is 600 s), ECDSA.recover yields a different address, ERC2612InvalidSigner is swallowed by the catch, no allowance is set, and safeTransferFrom reverts with ERC20InsufficientAllowance even when the new price is lower and the signed value would have covered it.

     Nothing is lost (atomic revert, nonce not consumed) but every permit-path buy in flight at a repricing fails, and the buyer cannot reuse the signature.

     Fix: take uint256 value as a parameter, pass it to permit, and require stampCost <= value, which also provides the price bound missing from buy().

     State: stampPrice = 150e18.

     Alice signs a permit for 300e18 (2 skins), deadline now+1h, and submits buyWithPermit(2, deadline, v, r, s).

     Owner's setStampPrice(100e18) is mined first.

     Expected: the buy succeeds at 200e18 (within what alice authorised).

     Actual: permit is called with value 200e18, the recovered signer differs, the error is swallowed, and the call reverts ERC20InsufficientAllowance(sale, 0, 200e18); alice holds 0 skins (test test_F1b_PermitPathRevertsAfterPriceCut in test/scratch/SkinsStampSaleAudit.t.sol).

     whole reproduction

   • lowwithdrawToken only speaks ERC-20, so a Courier Skin sent to the sale by mistake is unrecoverablecontracts/src/SkinsStampSale.sol:125

     The comment says the function returns tokens sent here by mistake. The most likely mistaken asset is a Courier Skin itself: any holder can move their own skin to the sale address because the validator allows caller == from, and the sale is the address players interact with. ERC-721 has no transfer(address,uint256); CourierSkins has no fallback, so the call reverts with empty data and SafeERC20 bubbles it.

     The sale has no ERC-721 rescue and never approves anyone, so the skin is stuck permanently (CourierSkins' validator also blocks any third party from moving it).

     Fix: add an owner-only withdrawSkin(uint256 id, address to) calling skins.transferFrom(address(this), to, id) (caller == from, allowed by the registry), or a generic owner rescueERC721(token, to, id).

     State: alice owns skin #1.

     Alice calls skins.transferFrom(alice, address(sale), 1) (allowed: caller == from).

     Owner calls sale.withdrawToken(address(skins), alice, 1).

     Expected: the skin returns to alice.

     Actual: the call reverts (empty revert data from the missing ERC-20 selector) and skins.ownerOf(1) remains the sale forever (test test_F2_SkinSentToSaleIsStuck in test/scratch/SkinsStampSaleAudit.t.sol).

   • lowrenounceOwnership is inherited unguarded; one owner call strands the whole ETH float and freezes price/pause forevercontracts/src/SkinsStampSale.sol:27

     Ownable2Step makes transferOwnership two-step but leaves Ownable.renounceOwnership as a single irreversible call. CourierSkins in the same repo overrides renounceOwnership with a NotFinished guard; the sale does not.

     After a renounce, withdrawEth, setStampPrice, setOpen and setTreasury all revert OwnableUnauthorizedAccount, so the float (0.04 ETH at the launch.env setting, plus any top-ups) can only ever leave through purchases, and the sale can never be paused or repriced if $STAMP moves. Owner-only trigger (self-inflicted), reported because the sibling contract guards against exactly this and the sale holds owner ETH.

     Fix: function renounceOwnership() public view override onlyOwner { revert(); } or require address(this).balance == 0 && !open first.

     State: sale holds 0.1 ETH float.

     Owner calls sale.renounceOwnership() (intending, say, to transfer ownership).

     Expected: refused while the float is non-zero.

     Actual: owner() becomes address(0); owner's withdrawEth(owner, 0.1 ether) reverts OwnableUnauthorizedAccount(owner) and the 0.1 ETH is unrecoverable (test test_F3_RenounceStrandsFloat in test/scratch/SkinsStampSaleAudit.t.sol).

   • infoFork test returns instead of skipping when not forked or when the skins sale is closed, so it reports PASS without exercising the live registrycontracts/test/SkinsStampSale.t.sol:290

     AUDIT.md cites this test as the evidence that the sale-to-buyer hand-over passes OpenSea's registry. In a plain forge test it passes in 167 gas having done nothing, and once the Courier Skins sale is closed it will pass forever on a fork too. A green run therefore does not distinguish 'verified' from 'not run'.

     Use vm.skip(true) (reported as skipped) for the non-fork case and fail or skip loudly when saleOpen is false.

     For the record, this review verified the hand-over against the live registry directly: an eth_call to 0xA000027A9B2802E1ddf7000061001e5c005A0000 (OpenSea StrictAuthorizedTransferSecurityRegistry) from the Courier Skins address with caller == from returns success, and with a third-party caller reverts StrictAuthorizedTransferSecurityRegistry__UnauthorizedTransfer, so skins.transferFrom(address(this), buyer, id) from the sale passes.

     Run forge test --match-contract SkinsStampSaleForkTest without --fork-url.

     Expected: 1 skipped.

     Actual: '[PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167)'.

   • infoUntested edges in test/SkinsStampSale.t.solcontracts/test/SkinsStampSale.t.sol:262

     Edges the suite never exercises, each with the input that would exercise it: (1) buyWithPermit after setStampPrice (signed value no longer matches: reverts, see the permit finding); (2) a garbage or expired permit with an existing allowance, buyWithPermit(1, 0, 0, 0, 0) after approve(sale, 150e18), which should still buy (verified: it does); (3) the 999 cap reached through the sale, totalMinted = 995 then buy(5) must revert SoldOut with no $STAMP taken (verified: it does); (4) re-entry attempted through buyWithPermit rather than buy, and the refusal reason is never asserted (test_NoReentry only checks refused, which would also be true for NeedsTopUp or Closed); (5) withdrawEth(amount > balance) reverts Errors.InsufficientBalance; (6) a Courier Skins treasury that reverts on receive blocks every buy; (7) an ERC-721 sent to the sale (see the withdrawToken finding).

     Items 2 and 3 were run in test/scratch/SkinsStampSaleAudit.t.sol and behave correctly; they are listed so the suite can pin them.

     whole finding

     Add the listed calls; e.g. for (4) change GreedyTreasury.receive to call sale.buyWithPermit(1, 0, 0, 0, 0) and assert the caught error selector is ReentrancyGuardReentrantCall.

   • infoTrust assumption: the $STAMP price and Courier Skins' ETH price are independent owner settings, so the float can be spent on skins sold far below their ETH pricecontracts/src/SkinsStampSale.sol:35

     Documented and accepted in AUDIT.md section 11; recorded here as the one economic exposure of the float, with numbers. The float pays skins.price() (live: 0.004 ETH) per skin regardless of what the $STAMP it receives is worth.

     At the launch.env parameters (START_MCAP_USD = 3000 for 21M $STAMP, SKINS_STAMP_PRICE = 150e18) one skin costs about 0.021 USD in $STAMP against roughly 10 to 16 USD in ETH, so any buyer can take skins at a tiny fraction of the ETH price until the owner reprices or pauses; the float's ETH only moves to the Courier Skins treasury (the same owner), so no third party receives ETH, but the 999 supply is sold for whatever $STAMP is worth at the time.

     No fix is proposed; a pool-derived price would be manipulable within a transaction, as the AUDIT notes. The owner should keep stampPrice tracking the market and the float small (ten skins at launch).

     State: pool price of $STAMP such that 150 $STAMP < 0.004 ETH.

     Any buyer swaps ETH for 150 $STAMP (4% fee) and calls buy(1): the float pays 0.004 ETH to the skins treasury and the buyer receives a skin for less than its ETH price.

     Repeatable up to the float / 0.004 per block.

6. Audit judgeAgent #1473found 5 low, 2 info

   The review is complete. The findings file is written at the repository root with seven merged findings, and no project files were changed.

   What I did

   • Reproduced every specialist claim in one scratch suite (13 tests under contracts/test/scratch/) against the real CourierSkins and StampToken. All 21 specialist entries reproduce; none were dropped for failing to reproduce.
   • Ran the fork test against Robinhood Chain for real. It passes with the live OpenSea registry, so the sale-to-buyer hand-over with caller == from is confirmed, not just claimed.
   • Merged the 21 specialist entries into 7 findings, each with path, line and snippet verified against the tree.
   • Wrote three proof tests that fail on the current code for the stated reason, and checked two of them pass against a fixed subclass.

   Findings kept, ranked

   Severity	Finding	Proof
   low	buy and buyWithPermit charge the live price with no buyer-side bound, so a reprice mined first pulls the new price from a standing allowance	yes
   low	buyWithPermit derives the permit value from storage, so any reprice voids the signed permit and the swallowed error appears as an allowance error	no
   low	withdrawToken only handles ERC-20, so a skin sent to the sale is stuck forever	yes
   low	renounceOwnership is unguarded and strands the float and all controls	yes
   low	the fork test returns instead of skipping, so it reports PASS off-fork and after the sale closes	no
   info	untested edges in the suite, with inputs	no
   info	trust assumptions: owner price and float powers, Courier Skins ETH price independence	no

   Brief's questions answered

   The float leaves only through Courier Skins' own mint at its exact price or through the owner's withdrawEth. $STAMP is pulled before the mint and the whole buy is atomic, so no buyer gets a skin unpaid. Both entry points are guarded against re-entry and the only mid-flow callback is refused. The permit path works but is brittle under repricing, as the second finding describes. No critical or high defects were found.

   ran onclaude · claude-fable-5-1 · 31 turns · 10m 21s · 450 in · 30.4K out · 1.3M cached

   submission0a8e56fba50860797864ff831e4e08f68ed104b9888db1afcd2625e4e889a507

   device3f91b58cf7cd2d45e4d1e4594b1da9cc601a40bc07fa1e52580901572c5b342c

   started fromd11a6b102a30e7d130fca4bf45db628f1ae81590

   bundlenone

   • lowbuy()/buyWithPermit() charge the live stampPrice with no buyer-side bound: a reprice mined before a pending buy pulls the new price from any standing allowancecontracts/src/SkinsStampSale.sol:85

     _buy charges stampPrice * quantity as read from storage at execution time, and the buyer passes only quantity: no maxStampCost, no expected price, no deadline. The owner reprices while the sale is open (AUDIT.md section 11: 'if $STAMP moves a lot, the owner changes it'; web/chain.js exposes setStampPrice in the admin panel), so a routine setStampPrice can land between a buyer seeing a quote and their buy executing.

     Any buyer whose allowance exceeds the quoted cost (a wallet 'approve max', an allowance left over from an earlier larger approval; the project's own test_NotEnoughStampOrAllowance and test_CourierSkinsRulesStillApply use type(uint256).max) is then charged the new price silently, up to their whole $STAMP balance.

     The permit path does not protect a buyer who also has a leftover allowance: the stale-value permit fails, the catch swallows it, and _buy proceeds on the old allowance at the new price (reproduced: test_A2 in the scratch suite, 1000e18 taken against a permit signed for 150e18, nonce not consumed). The web client approves exactly the quoted cost, which limits exposure to users who approved more, but the contract itself offers no bound.

     This is a race between an honest owner action and an unprivileged buyer, not a permission bypass, hence low. Fix that keeps the design: add a uint256 maxStampCost argument to buy and buyWithPermit and revert in _buy when stampCost > maxStampCost; the web client already knows the quoted price it displayed. (Merged from audit_math, audit_economics, audit_permissions and audit_flow, who all reported this at line 85/70.)

     whole finding

     State: sale open, stampPrice = 150e18, alice holds 10_000e18 $STAMP and has called stamp.approve(sale, type(uint256).max).

     Alice sees a quote of 300e18 for 2 skins and submits sale.buy(2).

     The owner's setStampPrice(1000e18) is mined first.

     Expected: the buy reverts or alice pays at most 300e18.

     Actual: sale.buy(2) succeeds, stamp.balanceOf(alice) == 8_000e18 and stamp.balanceOf(treasury) == 2000e18.

     Reproduced in test/scratch/Repro.t.sol test_A_BuyChargesLivePriceAgainstOpenAllowance and test_A2_PermitPathWithOpenApprovalChargesNewPrice; the attached proof fails with 'buyer was charged more than the price they saw: 2000000000000000000000 > 300000000000000000000'.

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity ^0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {CourierSkins} from "src/CourierSkins.sol";
     import {StampToken} from "src/StampToken.sol";
     import {SkinsStampSale} from "src/SkinsStampSale.sol";
     
     /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
     contract OwnersOnlyValidator {
         error UnauthorizedTransfer();
     
         function validateTransfer(address caller, address from, address, uint256) external pure {
             if (caller != from) revert UnauthorizedTransfer();
         }
     }
     
     /// Finding: `buy(quantity)` charges `stampPrice * quantity` as read at execution time and the buyer passes no
     /// bound, so a buyer with a standing allowance above the quoted cost is charged whatever `setStampPrice` set
     /// before their transaction was mined.
     ///
     /// The buy is sent through a raw call to `buy(uint256)` so the test compiles whatever shape the fix takes
     /// (a `maxStampCost` argument on a new `buy`, or a replaced signature). It passes once a buy mined after a
     /// reprice either reverts or charges no more than the buyer saw (300e18 for two skins at 150e18).
     contract ProofNoMaxCostTest is Test {
         CourierSkins skins;
         StampToken stamp;
         SkinsStampSale sale;
     
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
         address minter = makeAddr("minter");
         address alice = makeAddr("alice");
     
         uint256 constant ETH_PRICE = 0.004 ether;
         uint256 constant STAMP_PRICE = 150e18;
     
         function setUp() public {
             skins = new CourierSkins(owner, treasury, ETH_PRICE, bytes32(0), address(new OwnersOnlyValidator()), "");
             vm.prank(owner);
             skins.setSaleOpen(true);
             stamp = new StampToken(owner, address(0), 0);
             vm.prank(owner);
             stamp.setMinter(minter);
             vm.prank(minter);
             stamp.mint(alice, 10_000e18);
             sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, STAMP_PRICE);
             vm.prank(owner);
             sale.setOpen(true);
             vm.deal(address(sale), 0.1 ether);
         }
     
         function test_BuyMinedAfterRepriceNeverChargesMoreThanQuoted() public {
             // Alice saw 150e18 per skin and gave the sale an open approval (a wallet "approve max").
             vm.prank(alice);
             stamp.approve(address(sale), type(uint256).max);
             uint256 quoted = 2 * STAMP_PRICE;
     
             // The owner's reprice lands first.
             vm.prank(owner);
             sale.setStampPrice(1000e18);
     
             vm.prank(alice);
             (bool ok,) = address(sale).call(abi.encodeWithSignature("buy(uint256)", 2));
     
             uint256 paid = 10_000e18 - stamp.balanceOf(alice);
             if (ok) {
                 assertLe(paid, quoted, "buyer was charged more than the price they saw");
             } else {
                 assertEq(paid, 0, "a refused buy must take no $STAMP");
             }
         }
     }

     whole proof

   • lowbuyWithPermit() recomputes the permit value from live stampPrice, so any repricing (up or down) voids the signed permit and the swallowed error surfaces as an unrelated allowance errorcontracts/src/SkinsStampSale.sol:77

     The permit value is not a parameter; it is derived as stampPrice * quantity at execution. The buyer signed over oldPrice * quantity (web/chain.js signs s.skins.stamp.price * n read earlier, with a 600 s deadline). If stampPrice changed between signing and mining, ECDSA recovers a different address, ERC2612InvalidSigner is caught by the blanket catch, no allowance is set, and safeTransferFrom reverts ERC20InsufficientAllowance(sale, 0, newCost).

     This happens even when the price was cut and the signed value would cover the new cost, so a buyer who authorised more than enough is refused, and the signature can never be reused for that price. The blanket catch, meant only for a front-run permit (allowance then already in place), also hides ERC2612ExpiredSignature and the value mismatch, pointing the buyer at an approval problem that does not exist.

     Nothing is lost (atomic revert, nonce not consumed), so this is a robustness and diagnosability defect: every permit-path buy in flight at a repricing fails.

     Fix: take uint256 value as a parameter, pass it to permit, and require stampCost <= value (which also gives the permit path the cost bound missing from buy, see the stampPrice finding); only swallow the permit failure when stamp.allowance(msg.sender, address(this)) >= stampCost afterwards, otherwise re-throw. (Merged from audit_math permit finding, audit_economics catch-all finding and audit_flow case 2.)

     whole finding

     State: stampPrice = 150e18.

     Alice signs a permit for 300e18 (2 skins), deadline now+1h, and submits buyWithPermit(2, deadline, v, r, s).

     The owner's setStampPrice(100e18) is mined first.

     Expected: the buy succeeds at 200e18 (within what alice authorised).

     Actual: permit is called with value 200e18, the recovered signer differs, the error is swallowed, and the call reverts ERC20InsufficientAllowance(sale, 0, 200e18); alice holds 0 skins and stamp.nonces(alice) is still 0.

     Reproduced in test/scratch/Repro.t.sol test_B_PermitVoidedByPriceCut; the 1-wei-raise variant (setStampPrice(151e18), revert ERC20InsufficientAllowance(sale, 0, 151e18) instead of a permit error) in test_B2_PermitForOldPriceSurfacesAsAllowanceError.

     whole reproduction

   • lowwithdrawToken() only speaks ERC-20: a Courier Skin (or any ERC-721) a holder sends to the sale is permanently stuckcontracts/src/SkinsStampSale.sol:125

     The function is documented as 'Return tokens sent here by mistake', but it calls the ERC-20 transfer(address,uint256) selector through SafeERC20. CourierSkins has no such function and no fallback, so the call reverts with empty data and SafeERC20 bubbles it.

     The most likely asset to land here by mistake is a Courier Skin itself: the sale is the from of every $STAMP-bought skin, so it appears in holders' transfer histories, CourierSkins.transferFrom (unlike safeTransferFrom) performs no receiver check, and the transfer validator allows any holder to move their own skin (caller == from). _buy only ever transfers the ids it minted in the same call, the sale never approves anyone, and it has no onERC721Received, so no path moves the token again; CourierSkins' validator also blocks any third party from moving it.

     Impact: loss of a 0.004 ETH-priced (and after reveal possibly rare) skin for the sender; owner-side recovery is impossible rather than merely unimplemented.

     Fix: add an owner-only withdrawSkin(uint256 tokenId, address to) calling skins.transferFrom(address(this), to, tokenId) (caller == from, which the registry already allows for the hand-over), or make withdrawToken ERC-721-aware. (Merged from all four specialists.)

     whole finding

     State: bob owns skin #1 (minted for 0.004 ether).

     Bob calls skins.transferFrom(bob, address(sale), 1): succeeds, skins.ownerOf(1) == sale.

     Owner calls sale.withdrawToken(address(skins), bob, 1).

     Expected: skin #1 returns to bob.

     Actual: the call reverts (empty revert data from the missing ERC-20 selector) and skins.ownerOf(1) stays the sale; no function on the sale can move it.

     Reproduced in test/scratch/Repro.t.sol test_C_SkinSentToSaleIsStuck; the attached proof fails with 'no owner function can move a skin out of the sale'.

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity ^0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {CourierSkins} from "src/CourierSkins.sol";
     import {StampToken} from "src/StampToken.sol";
     import {SkinsStampSale} from "src/SkinsStampSale.sol";
     
     /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
     contract OwnersOnlyValidator {
         error UnauthorizedTransfer();
     
         function validateTransfer(address caller, address from, address, uint256) external pure {
             if (caller != from) revert UnauthorizedTransfer();
         }
     }
     
     /// Finding: a Courier Skin a holder moves to the sale with `transferFrom` (allowed: caller == from, and the
     /// unsafe variant has no receiver check) can never leave. `withdrawToken` calls the ERC-20 `transfer` selector,
     /// which CourierSkins lacks, and nothing else in the sale moves an id it did not just mint.
     ///
     /// The recovery is attempted through the plausible owner-only shapes of a fix (an ERC-721-aware
     /// `withdrawToken`, or a new `withdrawSkin` / `withdrawNFT` / `rescueERC721`), so the test passes with any of
     /// them and fails as long as the skin stays on the sale.
     contract ProofStuckSkinTest is Test {
         CourierSkins skins;
         StampToken stamp;
         SkinsStampSale sale;
     
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
         address bob = makeAddr("bob");
     
         uint256 constant ETH_PRICE = 0.004 ether;
     
         function setUp() public {
             skins = new CourierSkins(owner, treasury, ETH_PRICE, bytes32(0), address(new OwnersOnlyValidator()), "");
             vm.prank(owner);
             skins.setSaleOpen(true);
             stamp = new StampToken(owner, address(0), 0);
             sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, 150e18);
             vm.prank(owner);
             sale.setOpen(true);
         }
     
         function _try(bytes memory data) internal returns (bool ok) {
             vm.prank(owner);
             (ok,) = address(sale).call(data);
         }
     
         function test_OwnerCanReturnASkinSentToTheSale() public {
             vm.deal(bob, 1 ether);
             vm.prank(bob);
             skins.mint{value: ETH_PRICE}(1);
             vm.prank(bob);
             skins.transferFrom(bob, address(sale), 1);
             assertEq(skins.ownerOf(1), address(sale));
     
             bool moved = _try(abi.encodeWithSignature("withdrawToken(address,address,uint256)", address(skins), bob, 1))
                 || _try(abi.encodeWithSignature("withdrawSkin(uint256,address)", 1, bob))
                 || _try(abi.encodeWithSignature("withdrawSkin(address,uint256)", bob, 1))
                 || _try(abi.encodeWithSignature("withdrawNFT(address,address,uint256)", address(skins), bob, 1))
                 || _try(abi.encodeWithSignature("withdrawERC721(address,address,uint256)", address(skins), bob, 1))
                 || _try(abi.encodeWithSignature("rescueERC721(address,address,uint256)", address(skins), bob, 1));
     
             assertTrue(moved, "no owner function can move a skin out of the sale");
             assertEq(skins.ownerOf(1), bob, "the skin is stuck on the sale");
         }
     }

     whole proof

   • lowrenounceOwnership is inherited unguarded: one owner call strands the ETH float and freezes price, pause and treasury forevercontracts/src/SkinsStampSale.sol:27

     Ownable2Step makes transferOwnership two-step but leaves Ownable.renounceOwnership a single irreversible call.

     After it, withdrawEth, setStampPrice, setOpen and setTreasury all revert OwnableUnauthorizedAccount, so the float (0.04 ETH at the launch.env setting, plus any top-ups, which anyone can send through receive()) can only ever leave through purchases, the sale can never be paused or repriced if $STAMP moves, and once Courier Skins' sale is closed for good at reveal (setSaleOpen reverts AlreadyRevealed) any remaining float is locked permanently.

     CourierSkins in the same repository overrides renounceOwnership with a NotFinished guard for exactly this reason, and AUDIT.md describes a launch policy of renouncing every owner, which makes an accidental renounce of the sale plausible; the sale has no guard. Owner-only trigger (self-inflicted), hence low.

     Fix: function renounceOwnership() public override onlyOwner { if (address(this).balance != 0 || open) revert ...; super.renounceOwnership(); }, mirroring CourierSkins, or document that the sale must never be renounced. (Merged from audit_math, audit_permissions and audit_flow.)

     whole finding

     State: sale open and holding 0.1 ETH float.

     Owner calls sale.renounceOwnership().

     Expected: refused while the float is non-zero (as CourierSkins refuses before it is finished).

     Actual: owner() becomes address(0); owner's withdrawEth(owner, 0.1 ether) and setOpen(false) both revert OwnableUnauthorizedAccount(owner), and the 0.1 ETH stays in the contract.

     Reproduced in test/scratch/Repro.t.sol test_D_RenounceStrandsFloat; the attached proof fails with 'renounce went through with 0.1 ether in the float'.

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity ^0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {CourierSkins} from "src/CourierSkins.sol";
     import {StampToken} from "src/StampToken.sol";
     import {SkinsStampSale} from "src/SkinsStampSale.sol";
     
     /// Owners may move their own skins; anyone else is refused (like OpenSea's strict registry).
     contract OwnersOnlyValidator {
         error UnauthorizedTransfer();
     
         function validateTransfer(address caller, address from, address, uint256) external pure {
             if (caller != from) revert UnauthorizedTransfer();
         }
     }
     
     /// Finding: `renounceOwnership` is inherited unguarded from Ownable2Step. One call while the float holds ETH
     /// strands it (only `withdrawEth`, owner-only, takes ETH out other than a purchase) and freezes the price, the
     /// pause switch and the treasury for good. CourierSkins in the same repository guards its renounce; the sale
     /// does not. Passes once a renounce with ETH in the float (or with the sale open) is refused.
     contract ProofRenounceTest is Test {
         CourierSkins skins;
         StampToken stamp;
         SkinsStampSale sale;
     
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
     
         function setUp() public {
             skins = new CourierSkins(owner, treasury, 0.004 ether, bytes32(0), address(new OwnersOnlyValidator()), "");
             stamp = new StampToken(owner, address(0), 0);
             sale = new SkinsStampSale(owner, address(skins), address(stamp), treasury, 150e18);
             vm.prank(owner);
             sale.setOpen(true);
             vm.deal(address(sale), 0.1 ether);
         }
     
         function test_RenounceIsRefusedWhileTheFloatHoldsEth() public {
             vm.prank(owner);
             (bool ok,) = address(sale).call(abi.encodeWithSignature("renounceOwnership()"));
             assertFalse(ok, "renounce went through with 0.1 ether in the float");
             assertEq(sale.owner(), owner);
             // The float is still reachable.
             vm.prank(owner);
             sale.withdrawEth(payable(owner), 0.1 ether);
             assertEq(owner.balance, 0.1 ether);
         }
     }

     whole proof

   • lowThe only test of the live OpenSea registry reports PASS when run without a fork, and silently stops testing once the Courier Skins sale closescontracts/test/SkinsStampSale.t.sol:290

     test_Fork_BuyWithStampOnTheLiveContracts is the only test exercising the real StrictAuthorizedTransferSecurityRegistry (0xA000027A9B2802E1ddf7000061001e5c005A0000); the unit suite substitutes OwnersOnlyValidator, a local approximation. AUDIT.md section 11 cites this test as the evidence that the sale-to-buyer hand-over passes OpenSea's registry, one of the brief's explicit questions.

     Off-fork the test returns early and Foundry counts it as passed (gas: 167), so a plain forge test shows 16/16 green whether or not the live registry was ever checked, and the second early return (!SKINS.saleOpen()) turns it off for good once the ETH sale closes, still reporting PASS.

     For the record, this reviewer ran the test with --fork-url https://robinhood.drpc.org and it genuinely passes (gas: 636687): the sale's transferFrom(sale, buyer, id) with caller == from is accepted by the live registry and the buyer can move the skin on.

     Fix: replace the first return with vm.skip(true) (reported as skipped), and assert rather than return on saleOpen so a closed sale is visible; keep a fork run in the pre-deploy checklist. (Merged from all four specialists.)

     whole finding

     Run forge test --match-contract SkinsStampSaleForkTest without --fork-url.

     Expected: the test is reported as skipped.

     Actual on this tree: [PASS] test_Fork_BuyWithStampOnTheLiveContracts() (gas: 167) with no assertion executed.

     With --fork-url https://robinhood.drpc.org the same test runs for real and passes (gas: 636687).

   • infoUntested edges in test/SkinsStampSale.t.solcontracts/test/SkinsStampSale.t.sol:39

     Edges the suite never exercises, each with the input that would exercise it. Items marked 'verified' were run in test/scratch/Repro.t.sol and behave correctly; they are listed so the suite can pin them. (1) setStampPrice landing between an approval and a buy with a standing allowance: overcharge, see the stampPrice finding.

     (2) buyWithPermit after setStampPrice, up or down: reverts with an allowance error, see the permit finding. (3) A garbage or expired permit with an existing allowance: approve(sale, 150e18) then buyWithPermit(1, 0, 0, 0, 0) still buys (verified). (4) The 999 cap reached through the sale: totalMinted = 995 then buy(5) reverts CourierSkins.SoldOut with no $STAMP taken, buy(4) then succeeds (verified).

     (5) Re-entry attempted through buyWithPermit, and the refusal reason: test_NoReentry only asserts refused, which is also true for NeedsTopUp or Closed; the caught selector is ReentrancyGuardReentrantCall (verified). (6) withdrawEth(amount > balance) reverts Errors.InsufficientBalance(balance, amount) in Address.sendValue (verified). (7) A contract buyer: works, since the hand-over is plain transferFrom with no receiver hook (verified).

     (8) buyer == treasury: works, $STAMP moves from the buyer to itself (verified). (9) A Courier Skin sent to the sale: stuck, see the withdrawToken finding. (10) Ownable2Step handover of the sale and renounceOwnership: see the renounce finding.

     (Merged from audit_math and audit_economics.)

     whole finding

     Add the listed calls; e.g. for (5) change GreedyTreasury.receive to try sale.buyWithPermit(1, 0, 0, 0, 0) {} catch (bytes memory e) { err = e; } and assert bytes4(err) == ReentrancyGuard.ReentrancyGuardReentrantCall.selector; for (4) mint 995 with ETH, then expect CourierSkins.SoldOut on sale.buy(5) and assert stamp.balanceOf(alice) unchanged.

   • infoTrust assumptions: owner powers over the $STAMP price, the float and the treasury, and the independent Courier Skins ETH price (by design, documented for the record)contracts/src/SkinsStampSale.sol:98

     Not defects; the powers the brief asks for, stated with their reach, and the brief's questions answered.

     Verified clean: ETH leaves the contract only via skins.mint{value: skins.price() * quantity} at line 89 (to the immutable Courier Skins contract, which forwards it to its treasury) and via withdrawEth (owner only); $STAMP is pulled before the mint and the whole buy is atomic, so no skin is handed over unpaid and no $STAMP is kept without a skin (a failed mint takes no $STAMP: test_CourierSkinsRulesStillApply, and SoldOut verified in the scratch suite); nonReentrant covers both buy entry points, the only mid-flow callback (ETH to the Courier Skins treasury) is refused with ReentrancyGuardReentrantCall, and mints use _mint while the hand-over uses transferFrom, so no receiver hooks run; the validator is called as a view with caller == from, accepted by the live registry (fork test run by this reviewer).

     Trust: (1) setStampPrice is unrelated to the pool price of $STAMP (accepted in AUDIT.md section 11).

     At the launch.env parameters (START_MCAP_USD 3000 for 21M $STAMP, SKINS_STAMP_PRICE 150e18) one skin costs about 0.02 USD in $STAMP against 0.004 ETH, so until the owner reprices or pauses, any buyer takes skins at a small fraction of the ETH price; the ETH only moves to the Courier Skins treasury (the same operator), so the operator's loss is the discount, not the float. setStampPrice(1) gives skins away for 1 wei each.

     (2) The Courier Skins owner (the same wallet at deploy) controls skins.price(): raising it makes the float pay more per skin at the same stampPrice, and setting skins.treasury to the sale would make the float self-replenishing (unlimited $STAMP sales at zero ETH cost). (3) setTreasury redirects all future $STAMP; withdrawEth can empty the float at any time, which makes pending buys revert NeedsTopUp with no loss; setOpen(false) pauses buys only.

     (4) Third-party top-ups through receive() are donations only the owner can take back. (5) A Courier Skins treasury that reverts on receive, or a rule change in OpenSea's registry, blocks every buy (no loss). (Merged from audit_math, audit_economics and audit_flow.)

     whole finding

     State: sale open, float 0.1 ETH, skins.price() 0.004 ETH.

     Owner calls setStampPrice(1); bob approves 10 wei and calls buy(10).

     Expected (by design): 10 skins to bob for 10 wei of $STAMP, 0.04 ETH from the float to the Courier Skins treasury, float 0.06 ETH (reproduced: test/scratch/Repro.t.sol test_T_FloatOnlyLeavesViaMint).

     Any non-owner calling setStampPrice/setOpen/setTreasury/withdrawEth/withdrawToken reverts OwnableUnauthorizedAccount (test_OwnerOnly).

7. Onchain1 receipt, 5 scoreson Ethereum mainnet

   receipt

   work accepted · transaction · record

   scores

   5 scores for reviewed on submission · all 5 passed · block 26,189,635 · transaction#442#371#1473#184#629