Smart Contract Auditing: Reentrancy and Common Solidity Flaws

Solidity audit starting points: reentrancy and other frequent flaws, plus Slither and Mythril usage notes.

11 min read
ibrahimsql
2,130 words

Smart Contract Auditing: Reentrancy and Common Solidity Flaws#

With billions of dollars locked in DeFi protocols, smart contract security is critical. A single bug can drain a protocol's entire liquidity in seconds.

Common Vulnerabilities#

1. Reentrancy#

The most famous bug (The DAO Hack). It occurs when a contract calls an external contract before updating its own state. The external contract can call back into the original function recursively, draining funds.

Fix: Use the Checks-Effects-Interactions pattern or a ReentrancyGuard.

2. Integer Overflow/Underflow#

In older Solidity versions (<0.8.0), numbers could wrap around. uint8(255) + 1 = 0

Fix: Use Solidity 0.8.0+ (which has built-in checks) or SafeMath library.

3. Front-Running#

Miners or bots can see your transaction in the mempool and insert their own transaction before yours to profit (MEV).

Auditing Tools#

  • Slither: A static analysis framework for Solidity.
  • Mythril: A security analysis tool for EVM bytecode.
  • Foundry: A blazing fast toolkit for application development and testing.

The Audit Process#

  1. Manual Review: Reading line-by-line to understand logic.
  2. Automated Scanning: Running tools to catch low-hanging fruit.
  3. Unit Testing: Writing thorough tests for edge cases.
  4. Fuzzing: Throwing random data at the contract to break it.

Conclusion#

Smart contract auditing is a high-stakes game. There is no "undo" button on the blockchain.

A Walked Reentrancy Example#

// Vulnerable contract Vault { mapping(address => uint) public balances; function withdraw() external { uint bal = balances[msg.sender]; (bool ok, ) = msg.sender.call{value: bal}(""); require(ok); balances[msg.sender] = 0; // state update happens too late } }

The attacker contract calls withdraw, receives the ETH, and its receive() function calls withdraw again before balances is zeroed. Checks-Effects-Interactions inverts the order: zero the balance first, then send.

More Flaw Classes#

  • Unprotected selfdestruct: an old owner-only pattern without a proper onlyOwner modifier.
  • tx.origin auth: require(tx.origin == owner) fails when the call comes through a contract.
  • Delegatecall to untrusted libraries: lets the callee overwrite the caller's storage slots.
  • Signature replay: reusing a signed message across chains or forks without a nonce.

Reading a Foundry Project#

forge build forge test -vv slither . --detect reentrancy-eth,reentrancy-no-eth

Foundry tests double as attack scaffolding. Fork mainnet state with forge test --fork-url <rpc> and write the exploit as a test; the trace shows which storage slot moved.

Manual Review Order#

  1. Read the inheritance graph; most bugs hide in the base contract.
  2. Trace every call, delegatecall, and transfer.
  3. Check access modifiers on admin functions.
  4. Diff deployed bytecode against the verified source before trusting the repo.
  5. Compare storage layout for upgradeable (proxy) contracts; slot collisions corrupt funds.

Tool Notes#

  • Slither: fast static analysis; noise ratio is high, so triage by impact.
  • Mythril: symbolic execution over bytecode; slower, better for arithmetic edge cases.
  • Echidna: property-based fuzzing; write invariants like totalSupply == sum(balances).
  • Tenderly / Phalcon: transaction-level traces; handy when a finding needs a live repro.

Report Template#

FieldExample
SeverityHigh - unauthorized withdrawal
PreconditionsAttacker must hold a balance in the vault
StepsDeploy receiver contract, call withdraw, reenter in receive()
ImpactDrain of contract balance
FixMove state update before the call; add ReentrancyGuard

Keep one PoC per file. Auditors prefer a repro that runs with forge test over prose.

Access Control Failures#

contract Token { address public owner; constructor() { owner = msg.sender; } modifier onlyOwner() { require(msg.sender == owner); _; } function mint(uint amt) external onlyOwner { ... } function transferOwnership(address o) external { owner = o; } // no onlyOwner }

The second function is missing its guard. Every privileged function needs the same check, and the check must run on the caller, not on tx.origin.

Upgradable Contract Risks#

RiskMechanism
Storage collisionNew variable inserted before existing ones shifts slots
Selector clashTwo functions share a 4-byte selector
Uninitialized implementationAttacker initializes the logic contract and forks the proxy

Check the proxy's slot for the implementation address. In EIP-1967, that slot is bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1).

Gas Considerations#

Unbounded loops over user-controlled arrays turn into denial of service. withdraw() iterating a growing array eventually exceeds the block gas limit. Page through the state with offsets instead.

Fork Testing#

forge test --fork-url $MAINNET_RPC --fork-block-number 19000000

A fork lets you replay a real exploit against live balances without risking anything. Write the exploit as a Foundry test and keep the trace with the report.

Signing Flows#

EIP-712TypedData signatures need a domain separator that includes chainId. Without it, a signature captured on mainnet replays on a fork. Check that the verifier reconstructs the same domain hash the signer used.

Compiler Warnings#

forge build 2>&1 | grep Warning

Warnings like "unused return value" often mark a missed error check. They deserve the same triage time as a Slither high.

Audit Checklist#

  • Every external call's ordering matches Checks-Effects-Interactions.
  • Access modifiers present on all admin paths.
  • No reliance on block.timestamp for randomness.
  • No unchecked IERC20 transfer assumptions.
  • Events emitted for state changes that matter to users.

Storage Slot Basics#

Each state variable lives in a 32-byte slot. uint8 packs with neighbors. When you read a proxy storage collision, count slots from the implementation and from the proxy. Two variables landing in the same slot corrupt each other.

// Instance A expects slot 0 to hold owner. // Instance B expects slot 0 to hold totalSupply. // A collision overwrites owner with supply math.

Event Integrity#

Emit an event for every privileged action. Silent admin calls are hard to audit. Check that buyers can verify off-chain accounting through events alone.

Randomness#

Never use block.timestamp or blockhash for a prize or an ID that matters. Both are miner-influenced within a bounded window. Use a commit-reveal scheme or a VRF like Chainlink's.

Token Edge Cases#

  • ERC-777 tokensToSend hook breaks reentrancy-safe assumptions.
  • Fee-on-transfer tokens make balanceOf deltas unequal to sent amounts.
  • USDT returns no boolean from transfer; assume missing return values.
  • Approve-zero race: old approve must be zeroed before a new nonzero.

Mempool Reality#

A signed transaction is visible before it mines. Sandwich attacks bracket a swap with front and back fills. Use deadline parameters and slippage bounds; never ignore them.

Audit Workflow Summary#

Read contracts in inheritance order. Mark every external call. Trace the effect on storage. Note privileged roles and their modifiers. Write the PoC for at least one finding before calling the audit done.

ERC-721 Approval Pitfalls#

setApprovalForAll grants full transfer rights. An audit of an NFT app should flag any place where a contract asks for blanket approval without scoping it to a specific listing. A compromised dApp can move every approved asset.

Proxy Layout Invariant#

Keep base storage slots identical between versions. New state variables go at the end, never inserted in the middle. Document the layout before shipping the upgrade; diff the deployed layout after.

Runtime Checks#

Use solc warnings, Slither, and a fuzzer on the diff. No single tool catches everything. The report's value is the combination.

Dry Run Checklist#

Before signing off on a review, run through:

  • Inheritance graph read end to end
  • External call ordering documented
  • Admin paths checked for modifiers
  • Storage layout compared to the previous version
  • Event emission for privileged actions confirmed
  • One finding reproduced as a Foundry test

Reference Tables#

ConcernCheck
ReentrancyState update before external call
Access controlModifier on every admin function
Upgrade safetyStorage layout matches prior version
RandomnessNo block.timestamp for values
Token behaviorFee-on-transfer and approvals audited
EventsEmitted for privileged transitions
Flash loansOracle reads gated
SelfdestructGuarded and non-selfdestructable path

Reminders#

  1. confirm before reporting
  2. confirm before reporting
  3. confirm before reporting
  4. confirm before reporting
  5. confirm before reporting
  6. confirm before reporting
  7. confirm before reporting
  8. confirm before reporting
  9. confirm before reporting
  10. confirm before reporting
  11. confirm before reporting
  12. confirm before reporting
  13. confirm before reporting
  14. confirm before reporting

Command Cheatsheet#

forge build forge test -vv slither . mythril analyze contract.sol

Final Notes#

  1. confirm, document, report
  2. confirm, document, report
  3. confirm, document, report
  4. confirm, document, report
  5. confirm, document, report
  6. confirm, document, report
  7. confirm, document, report
  8. confirm, document, report
  9. confirm, document, report
  10. confirm, document, report
  11. confirm, document, report
  12. confirm, document, report
  13. confirm, document, report
  14. confirm, document, report
  15. confirm, document, report
  16. confirm, document, report
  17. confirm, document, report
  18. confirm, document, report
  19. confirm, document, report
  20. confirm, document, report

How The Page Fits Together#

This page is the quick-reference chapter: start with the mechanism, drop into the code samples, then audit the checklist. Each finding from the walkthrough above maps to a row in the summary table.

StageArtifact
ReconVerified source match
AuditPer-function notes
FuzzInvariant failures
ReportOne PoC per finding

Closing Notes#

  1. Verify each observation with a second independent check.
  2. Prefer packet, request/response, or log evidence over prose.
  3. Tie the finding to the control that should have caught it.
  4. Redact secrets but keep the request shape visible.
  5. Never quote a payload you have not actually tested.
  6. Keep one repro per file; name it clearly.
  7. Update the matrix when a new tool or a new gadget appears.
  8. Shorten CI runs; the tool should fit in a normal deploy.
  9. Confirm a false positive before you report it.
  10. A finding without evidence is folklore.
  11. Document the exact time delta or row count.
  12. Record the binary version or framework version in the report.
  13. Every tool output in the report ties to a command.
  14. The evidence tree should let a second reader replay the finding.
  15. Log the encoding and the raw bytes with the finding.
  16. If a fix closes one probe, re-run the others to be sure.
  17. Closing one instance rarely closes the class.
  18. Rotate pretexts and rate limits so the test keeps signal.
  19. Track report rate, not click rate, for awareness campaigns.
  20. Keep capture files short; slice the interesting window.
  21. ZAP baselines belong in CI, full scans on staging.
  22. Burst the auth endpoint, then verify no lockout.
  23. Diff lockfiles before running a supply-chain claim.
  24. Parameter pollution hides in headers and cookies too.
  25. Checksum your dependencies and rotate keys on a schedule.
  26. Time-based blind is slow; always pair it with a boolean check.
  27. Show the GRANTS output; it proves reachability limits.
  28. DOM sinks are data-flow endpoints, not just payload targets.
  29. Trusted Types and CSP sit on the same defense line.
  30. Stash the tshark JSON slice with the pcap for reference.
  31. A short pcap with notes beats a ten-minute capture.
  32. Name files with date, host, and window.
  33. A flat periodic line in the IO graph is a beacon candidate.
  34. Spike bursts in Burp are usually manual, not scanner.
  35. Tune Nuclei rate limits so the range is not blocked.
  36. sqlmap risk/level flags widen the payload classes.
  37. Arjun finds parameters that do not appear in the URL.
  38. Keep WordPress plugin nonces in a separate test case.
  39. XML-RPC is the slow, quiet way in. Disable it if unused.
  40. Backup files in the docroot are findings by themselves.
  41. Upload extension checks should run on the normalized name.
  42. Homoglyphs hide inside scope lists and allowlists.
  43. Normalize at the edge, log raw, never filter before decode.
  44. UTF-7 and legacy codecs keep bypassing naive filters.
  45. A fullwidth probe that passes is a bug in the pipeline.
  46. Rotate keys, pin versions, and rotate them again after a patch.
  47. Prototype pollution turns one key into a global change.
  48. __proto__ is the first key to block; check constructor too.
  49. Run npm ls deep; the vulnerable path is rarely the top level.
  50. Refuse constructor.prototype keys at every merge point.
  51. Freeze Object.prototype for the server if you must merge.
---
Share this post:

What do you think?

React to show your appreciation

Related Posts