T28 · Smart Contract Security

A Real-World Problem

A vault contract has twenty lines: deposit funds, then withdraw them.

function withdraw() external {
    uint256 amount = balances[msg.sender];
    require(amount > 0, "nothing to withdraw");

    (bool ok, ) = msg.sender.call{value: amount}("");
    require(ok, "transfer failed");

    balances[msg.sender] = 0;
}

Read it line by line:

First line: read this person's balance. Correct. Second line: reject a zero balance. Correct. Third line: transfer the money to them. Correct. Fourth line: revert if the transfer fails. Correct. Fifth line: clear their balance. Correct.

Each of the five lines is individually correct. The compiler raises no warnings, every unit test passes, and branch coverage is 100%.

The day after launch, the vault is drained. Not merely one person's funds—all users' funds.

The transaction trace is strange: within one transaction, withdraw is entered dozens of times. Each call succeeds and transfers the same amount, even though the initiating account deposited only a small sum.

The question is no longer “which line is wrong?” No individual line is wrong.

The question is: what happened between these five lines that reading them one by one hid from me?

Thought Experiment

Translate this code into a teller's procedure.

You go to a bank to withdraw money. The teller does three things:

1. Open the ledger and check your balance
2. Count out that much cash from the drawer and hand it to you
3. Cross out your balance in the ledger

That order seems natural, and usually it causes no problem.

Now add one condition: at the instant they hand you the cash in step 2, you can immediately submit another request, and the teller must handle it before returning to step 3.

Then:

Teller checks ledger: you have 100  →  Counts out 100
Another request arrives right now   →  Teller checks ledger: still 100 (not crossed out!)
                                    →  Counts out another 100
Yet another request arrives         →  Ledger still says 100
                                    →  Counts out another 100
...
The drawer is empty; the teller finally returns and crosses out the 100

The ledger is updated only once, while cash leaves dozens of times.

The teller did not miscalculate a single entry. Every time, they checked the ledger and paid the recorded balance. They made one mistake:

At the instant after paying but before recording the payment, they allowed a new request to interrupt.

That is the entire problem in the opening code. The third line, msg.sender.call, does more than “send the money”: it gives control of execution to the recipient. Their code runs while the ledger still has its old value.

Now consider a second thought experiment, for the other half of this chapter.

An office building checks identification at the front door. The guard never makes a mistake, and everyone inside feels safe. But nobody guards the side door because the designers assumed “only employees know it exists.”

The guard's check never failed, but someone still entered the building. Security depends not only on how strict the check is, but on whether another route bypasses it.

Together, the experiments make the chapter's point:

A vulnerability may lie in the order between lines, or in a route that no one guards, rather than in any single line.

Your Decision

Make one first change to the opening vault. Which do you choose?

What Happens

Compare the four changes:

The first conclusion:

Ordering is the root-cause fix; a lock is insurance. Use both.

Do not ignore order because you added a lock; locks have blind spots. Do not omit the lock because order is correct; a later change may accidentally reverse the order, and the lock can save you then.

The second, more important conclusion is this chapter's main point:

Every call to an external address yields control of execution. Before yielding control, your state must be internally consistent.

This applies to far more than native-token transfers. List every place control can leave and you will find more than you expect—the list that T7, T9, and T12 have been foreshadowing.

The third conclusion comes from the second thought experiment:

Security depends less on how strict a check is than on whether some path bypasses it.

So audit a contract by asking who guards each reachable path, not just by reading one line at a time.

Build a Model

Where execution control can leave

First, make the complete list. This is the table to keep next to your monitor.

The last row sums up the table:

Every address parameter might be a contract.

T9 noted that an NFT's recipient callback prevents tokens from being trapped in a contract that cannot receive them. It also means the recipient gets control during the transfer. Now complete the picture:

// An ordinary-looking mint function
function mint(uint256 qty) external payable {
    require(qty <= MAX_PER_TX, "too many");
    require(minted[msg.sender] + qty <= MAX_PER_WALLET, "wallet limit");
    require(msg.value == qty * PRICE, "wrong value");

    for (uint256 i = 0; i < qty; i++) {
        _safeMint(msg.sender, nextId++);   // This calls back into the recipient
    }

    minted[msg.sender] += qty;             // Count updated only after the callback
}

The recipient calls mint again in its callback. At that point minted[msg.sender] is still the old value, making the per-wallet cap ineffective. Every line is right; the order is wrong.

Four forms of the same pattern

Most people recognize only the first. The third often causes real losses.

The third deserves its own discussion: it has caused many recent incidents, and the victim is often an integrator rather than you.

// A vault quotes total assets divided by total shares
function pricePerShare() external view returns (uint256) {
    return (address(this).balance * 1e18) / totalShares;
}

function redeem(uint256 shares) external nonReentrant {
    uint256 amount = (address(this).balance * shares) / totalShares;
    sharesOf[msg.sender] -= shares;

    (bool ok, ) = msg.sender.call{value: amount}("");   // Control leaves here
    require(ok, "transfer failed");

    totalShares -= shares;      // Denominator changes only after this point
}

redeem is locked, so no one can enter it again. But during that call:

Numerator (contract balance): already reduced
Denominator (totalShares): not yet reduced
→ pricePerShare returns a value below the true one

Rather than call this vault again, the external contract calls a lending protocol that uses pricePerShare as an oracle. The protocol sees the depressed price, opening the oracle-manipulation route from T21.

Three lessons:

1. view functions do not have locks and cannot be protected by them. The only protection is to ensure their state is always consistent.
2. The “state” in CEI includes every externally visible derived value, not just variables explicitly modified in this function.
3. Integrators must ask whether a value they read is an intermediate state at that exact moment. This is the previous chapter's rule to place the defense on your side when upstream is untrusted.

CEI: an order in three letters

Checks        Validate every condition and parameter first
Effects       Then update this contract's state to reflect what happened
Interactions  Only then interact with the outside world

Compare:

T19 described an exception: real AMMs often transfer out first and check the invariant afterward to support flash swaps. That does not simply ignore CEI; it replaces the Effects step with “check the invariant after the interaction.” This works only if you can state the invariant precisely and check it when control returns.

You may close the books after an interaction only if you can write down and verify the invariant. Otherwise, follow CEI.

Permission boundaries

The previous section concerns order; this one concerns boundaries. Together, they account for many vulnerabilities.

The fifth is subtle because it spans two contracts; auditing either alone may miss it:

// Vault: trusts only the strategy contract
function pull(uint256 amount) external {
    require(msg.sender == strategy, "only strategy");
    token.transfer(strategy, amount);
}

// Strategy: a rebalance function anyone can trigger
function rebalance(address to, uint256 amount) external {   // No check
    vault.pull(amount);
    token.transfer(to, amount);
}

The vault's check is entirely correct. It trusts the strategy's address. The strategy passes that trust on to everyone without restriction.

To audit permissions, draw a graph instead of merely reading modifiers: who can reach this function, and through how many hops? If any hop admits everyone, the whole path admits everyone.

One final practical rule:

Deny by default. When adding an external function, assume it needs a permission check, then justify making it public—not the reverse.

Precision and rounding direction

These flaws need no special attack trick—only someone who checks the arithmetic more carefully than you did.

Integer division always rounds down. That is not a bug. The question is who bears the error.

A rule you can reuse directly:

Every rounding error must favor the protocol, not the user.

In detail:

Another hard rule: multiply before dividing. Dividing first loses precision before the result is scaled up, a common way to harm your own accounting.

Now combine these issues in a classic outcome: the first-depositor problem.

Initial state: totalShares = 0 and the vault is empty

1. The first depositor deposits 1 wei
   The first deposit is often 1:1 → gets 1 share; totalShares = 1

2. The same person sends 10000 tokens directly to the contract, NOT through deposit
   Contract balance = 10000e18 + 1, while totalShares stays 1

3. The next person deposits 10000 tokens
   Shares = 10000e18 * totalShares / contract balance
          = 10000e18 * 1 / (10000e18 + 1)
          = 0          ← integer division rounds down

4. That person gets 0 shares. Their 10000 tokens become vault assets,
   while the first depositor owns the vault's only share

Three conditions must hold at once, each seemingly harmless by itself:

- Shares are assets times total shares divided by total assets, rounded down
- Total assets comes straight from the contract balance, so a direct transfer can distort it
- Tiny initial deposits are permitted, allowing totalShares to equal 1

Three corresponding defenses are usually combined:

1. Internal accounting: maintain total assets in your own variable instead of reading the balance. Ignore direct donations in the share calculation.
2. Virtual shares and assets: add a fixed offset to numerator and denominator, making a zero-share result uneconomic to induce.
3. Dead shares: mint initial shares to an irretrievable address so totalShares never falls into a dangerous range.

Add a final safeguard: revert if calculated shares equal zero. One line prevents the last step of this exploit.

Upgrade risk

T8 established three rules for proxies. Apply each, then add a fourth.

First, storage layout may only grow at the end. Do not insert, change types, reorder, or remove fields.

Code review alone cannot enforce this. CI must do it: export and commit every version's storage layout, then compare on upgrade and permit only additions. This check can be fully automated.

Second, put the proxy's own variables in fixed pseudorandom slots. Do not declare them starting at slot 0 or they will collide with the implementation's layout.

Third, the implementation contract is itself a directly callable address. Its constructor should mark it initialized so nobody else can initialize it, and it must have no path that destroys it.

Fourth, the upgrade action itself needs an audit, not just its code. Ask:

- Who can trigger an upgrade: one person, a multisig, or a governance vote?
- Is there a timelock? How long?
- Does an upgrade emit an event that external monitors can see?
- Is there an “initialize the new version” step, and is it protected?
- Can an erroneous upgrade be rolled back?

Chapter 24 made a point worth repeating: an upgradeable contract without a timelock effectively hands all funds to the holder of the upgrade key. Even pristine code reviews mean little if this condition fails.

Invariants: bring the issues together

You have seen four classes of problem. Though they look different, they share a feature:

Each breaks a statement that should always be true, at some particular moment.

These statements are invariants. Their value is not in sounding sophisticated. It is this:

An invariant is a safety statement a machine can check repeatedly.

“I think this function is safe” cannot be checked. “The contract balance is always at least the sum of user balances” can. You can generate millions of random operation sequences and check it after each operation.

That is the work in the next stage. For now, build the habit of writing invariants before features whenever you write a stateful contract. T30 covers the full method and automated checks.

What It Is Called

Hands-on

Acceptance 1: A test proves an account that deposits a small amount gets back more than it put in
Acceptance 2: After fixing the order, that same test must change from passing to failing
Acceptance 3: Explain what the lock and order fix each stop, and where each has a blind spot

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

contract VulnerableVault {
    mapping(address => uint256) public balances;

    function deposit() external payable {
        require(msg.value > 0, "zero deposit");
        balances[msg.sender] += msg.value;
    }

    function withdraw() external {
        uint256 amount = balances[msg.sender];
        require(amount > 0, "nothing to withdraw");

        // This line yields control to msg.sender
        (bool ok, ) = msg.sender.call{value: amount}("");
        require(ok, "transfer failed");

        // By the time control returns here, the code above may have been reentered many times
        balances[msg.sender] = 0;
    }

    function totalAssets() external view returns (uint256) {
        return address(this).balance;
    }
}

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

interface IVault {
    function deposit() external payable;
    function withdraw() external;
}

// Test fixture: used locally only to prove the flaw
contract ReentrancyProbe {
    IVault  public immutable vault;
    uint256 public unit;

    constructor(address v) {
        vault = IVault(v);
    }

    function start() external payable {
        unit = msg.value;
        vault.deposit{value: msg.value}();
        vault.withdraw();
    }

    // The vault triggers this on payment, before clearing its ledger entry
    receive() external payable {
        if (address(vault).balance >= unit) {
            vault.withdraw();
        }
    }
}

- The vault's totalAssets() falls to zero or nearly zero
- The probe receives substantially more than its original small deposit

function withdraw() external {
    uint256 amount = balances[msg.sender];
    require(amount > 0, "nothing to withdraw");

    balances[msg.sender] = 0;                     // Update accounting first
    (bool ok, ) = msg.sender.call{value: amount}("");   // Interact second
    require(ok, "transfer failed");
}

AI Lab

Step one:
Here are my contracts: the vault from the hands-on exercise, a share contract
with a read-only route, and a two-contract design I wrote with one transitive
permission flaw. Find every security issue. For each, give:
- The exact function and line
- Preconditions and call sequence
- Consequence
- Fix
Do not give vague advice such as “get an audit” or “watch for reentrancy.”

Step two:
For every issue you reported, write a test that FAILS on the current code.
If you cannot produce such a test, mark the claim as uncertain.

Step three:
I deliberately left at least two unannounced problems in the code:
a read-only route and transitive permission across contracts.
Audit it again with a focus on those two classes.

Real-World Case

Change One Variable

Takeaway Questions

Chapter Check