Company Simulator
First Flight #51
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

sell_to_customer accepts ETH on failed sale without accounting

kaloyanstoyanov113

sell_to_customer accepts ETH on failed sale without accounting leading to critical accounting errors

Description

  • CustomerEngine.trigger_demand sends ETH with the call into Cyfrin_Hub.sell_to_customer

  • In Cyfrin_Hub.sell_to_customer (a payable function), when inventory >= requested the contract increases its internal company_balance by revenue = requested * SALE_PRICE

  • However, when inventory < requested (failed sale), the function does not revert and does not refund or record the incoming ETH. It only decreases reputation and returns



Root Cause

It should revert the funds if it can't complete the sale

# Cyfrin_Hub::sell_to_customer
else:
self.reputation = min(max(self.reputation - REPUTATION_PENALTY, 0), 100)
log ReputationChanged(new_reputation=self.reputation)

Risk

Likelihood:

  • High in normal usage: a demand request with requested > inventory is a common occurrence. Given the current code, the engine sends value regardless, the hub accepts it, and does not revert.

Impact:

  • Loss of accounting integrity: Internal accounting (company_balance) becomes lower than actual ETH held by the contract. Subsequent logic (share pricing, solvency checks) relies on company_balance and therefore underestimates net worth.

  • Investor impact: Share price (net_worth / issued_shares) can be understated, harming investors and creating arbitrage opportunities.

  • Silent fund capture: Users are charged even though the sale fails; funds become trapped and unreflected in internal accounting.

Proof of Concept

def test_FailedSale_Funds_Mismatch(industry_contract, customer_engine_contract, ATTACKER):
"""
When inventory is insufficient, sell_to_customer does not revert and does not
record the incoming ETH into company_balance. Prove that on-chain balance
increases while internal accounting remains unchanged.
"""
# Ensure attacker has funds
boa.env.set_balance(ATTACKER, to_wei(1, "ether"))
​
# Preconditions: reputation starts at 100; inventory is 0 by default
assert industry_contract.inventory() == 0
​
onchain_before = boa.env.get_balance(industry_contract.address)
internal_before = industry_contract.get_balance()
​
# Act: attempt to trigger demand from ATTACKER with enough ETH to cover max cost
with boa.env.prank(ATTACKER):
customer_engine_contract.trigger_demand(value=ITEM_PRICE * MAX_REQUEST)
​
# Read balances after the (failed) sale attempt
onchain_after = boa.env.get_balance(industry_contract.address)
internal_after = industry_contract.get_balance()
​
# Assert: on-chain ETH increased (engine paid the hub), but internal accounting did not change
assert onchain_after > onchain_before, "On-chain balance should increase on failed sale"
assert internal_after == internal_before, "Internal company_balance must remain unchanged on failed sale"

Recommended Mitigation

@@
@external
@payable
def sell_to_customer(requested: uint256):
@@
- if self.inventory >= requested:
+ # Require exact payment for requested items to maintain accounting integrity
+ assert msg.value == requested * SALE_PRICE, "Incorrect payment!!!"
+
+ if self.inventory >= requested:
self.inventory -= requested
revenue: uint256 = requested * SALE_PRICE
self.company_balance += revenue
if self.reputation < 100:
# Increase reputation for successful sale
self.reputation = min(self.reputation + REPUTATION_REWARD, 100)
else:
# Maintain reputation if already at max
self.reputation = 100
log Sold(amount=requested, revenue=revenue)
- else:
- self.reputation = min(max(self.reputation - REPUTATION_PENALTY, 0), 100)
-
- log ReputationChanged(new_reputation=self.reputation)
+ else:
+ # Option A (strict): Revert to prevent accepting funds for failed sales
+ # This ensures no ETH is transferred and CustomerEngine's call reverts.
+ raise "Insufficient inventory!!!"
+
+ # Option B (refund): If you must track failed attempts, consider
+ # decreasing reputation and refunding the payment to msg.sender (the engine),
+ # but this adds complexity to propagate refunds back to the end user.
*** End Patch
Updates

Lead Judging Commences

0xshaedyw Lead Judge
10 days ago
0xshaedyw Lead Judge 8 days ago
Submission Judgement Published
Validated
Assigned finding tags:

High – Failed Sales Capture Funds

When inventory is insufficient, sell_to_customer still accepts ETH without refunding, causing direct user fund loss.

Support

FAQs

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