3.9.1 Internal Audit Process
Internal auditing is the security work the development team does on its own code, before any external reviewer is engaged. It is not a smaller version of an external audit — it serves a different purpose. External audits answer the question "what did we miss?"; internal audits answer the question "is this ready to be reviewed?". A codebase that has not been internally audited is rarely ready for external review; an external audit performed on unprepared code produces findings that internal review should have caught, at the rate of senior security expert hours.
The internal audit process is also the most repeatable. External audits happen at milestones — a major release, a significant refactor, a new product launch. Internal audits should happen continuously: at every pull request, at every release candidate, and as a structured pass before each external engagement. The goal is to make the external audit a verification rather than a discovery.
This subsection covers the four pillars of internal auditing as a development practice: peer review, automated tooling, threat modeling, and test coverage. Each is examined as a workflow, with concrete tooling references and integration patterns.
Peer Code Review
Peer review is the lowest-overhead and highest-impact internal audit practice. Two developers reading each other's code catch a substantial fraction of the bugs that automated tools cannot, especially business-logic bugs that depend on understanding what the code is supposed to do.
The discipline that makes peer review effective:
One reviewer per pull request, minimum. For any non-trivial change touching value-handling logic, two reviewers. For changes to access control, upgradeability, or core invariants, the entire security-aware portion of the team.
Reviewers read the whole change, not just the diff. A PR that modifies three lines may have implications far outside those three lines. Reviewers should ask: what functions call this? What invariants does this affect? What tests exercise this path?
The reviewer's job is to find bugs, not to praise the work. Code review is not a celebration. A review with no comments is a review that wasn't performed. Every line that could plausibly be wrong should be questioned, even if the question is rhetorical.
Track findings, not just approvals. "LGTM" with no engagement is not a review. Productive reviews leave a trail of comments, even if the comments are answered satisfactorily and the PR is approved. This creates institutional knowledge over time.
Review Checklist
For every code review, the reviewer should explicitly verify:
- Access control on every state-changing function. Does this function have the appropriate modifier? Could an unauthorized caller execute it?
- External calls and their failure modes. What happens if the call reverts? What happens if it returns false? What happens if it consumes all forwarded gas?
- Arithmetic with user-controlled values. Can the inputs cause overflow, underflow, or precision loss? Is rounding direction correct?
- State changes before external calls (CEI compliance). If the function makes an external call, are all state changes completed first?
- Reentrancy exposure. Is this function reentrancy-resistant? Is the appropriate guard applied?
- Event emission for state changes. Does every state change emit an event? Are indexed parameters chosen well for downstream filtering?
- Test coverage for the changed code. Are there both positive and negative tests for the new logic? Does the test cover the failure modes the reviewer just imagined?
For value-handling protocols, this checklist should be enforced as a comment template in the PR system. A review that doesn't address each item is incomplete.
Automated Tooling
Automated tools cannot replace human review, but they catch bugs reliably and at near-zero marginal cost. The cost of running them is small; the cost of not running them and shipping a bug they would have caught is enormous. The discipline is to integrate them into the workflow so they run automatically rather than relying on developers to remember.
Static Analysis: Slither
Slither (from Trail of Bits) is the most widely-used Solidity static analyzer. It detects a broad catalog of known vulnerability patterns — uninitialized state variables, missing-modifier patterns, reentrancy candidates, dangerous strict equality, shadowing, and many others.
# Install
pip install slither-analyzer
# Run against the project
slither .
# Run with specific detectors filtered to a severity level
slither . --filter-paths "lib|test" --include-paths "src" --detect "high,medium"
# Output a SARIF report for CI integration
slither . --sarif report.sarif
Slither's detectors are categorized by severity (high, medium, low, informational). For new projects, run all detectors and triage every finding. For mature projects, integrate Slither into CI and fail the build on new high/medium findings.
Common false positives. Slither flags many patterns that are intentional in context — uninitialized state variables that get set in initialize(), reentrancy candidates that are protected by guards, etc. The right response is --triage-mode to acknowledge findings as understood. Suppressing findings without understanding them is the wrong response and leaks back into "we don't actually run Slither anymore."
Symbolic Execution: Mythril
Mythril performs symbolic execution at the bytecode level, exploring execution paths and detecting vulnerabilities like integer overflow (in pre-0.8 code), unchecked return values, and authorization issues. It's slower than Slither but catches different bugs.
# Install
pip install mythril
# Analyze a single contract
myth analyze src/MyContract.sol --solv 0.8.20
# Analyze with specific execution time limits
myth analyze src/MyContract.sol --solv 0.8.20 --execution-timeout 600
Mythril is most useful for contracts with complex control flow where Slither's pattern matching may miss subtle issues. It's not a default-on tool — run it before major releases rather than on every commit.
Compiler Diagnostics: solc with --warn
The Solidity compiler itself emits warnings that should not be ignored. Variable shadowing, unused variables, missing visibility, and many other patterns are flagged by the compiler.
# Compile with all warnings
solc --pretty-json --combined-json abi,bin,bin-runtime --optimize-runs 200 src/MyContract.sol
The solc warnings are noisy by default but every category is meaningful. Configure the project's build to treat warnings as errors in CI:
# foundry.toml
[profile.default]
deny_warnings = true
Most modern Solidity tooling (Foundry, Hardhat) supports treating warnings as errors. Enable this and treat new warnings as PR-blockers.
Linting: Solhint
Solhint enforces style and security conventions. Naming, file structure, contract size, missing NatSpec — Solhint catches the consistency issues that human reviewers would otherwise need to flag manually.
# Install
npm install -D solhint
# Run
npx solhint 'src/**/*.sol'
A baseline .solhint.json for security-conscious projects:
{
"extends": "solhint:recommended",
"rules": {
"no-inline-assembly": "warn",
"reason-string": ["warn", { "maxLength": 64 }],
"compiler-version": ["error", "0.8.20"],
"func-visibility": ["error", { "ignoreConstructors": true }],
"no-empty-blocks": "warn",
"private-vars-leading-underscore": "warn",
"no-shadowing": "error"
}
}
Solhint should run on every commit via a pre-commit hook or in CI.
Coverage: forge coverage
Coverage tools answer "which lines and branches of my code are tested?" — and the answer is almost always lower than developers think. Foundry's forge coverage produces a per-file report:
# Generate coverage
forge coverage --report lcov
# Generate a human-readable summary
forge coverage --report summary
A baseline goal for production code: 100% line coverage and 100% branch coverage for any function that modifies state or handles value. Achieving 100% is harder than it sounds — every revert path, every error condition, every modifier check needs to be exercised by a test. The discipline of pursuing 100% surfaces gaps that the developer would otherwise not notice.
Coverage is necessary but not sufficient. 100% line coverage with shallow tests proves the lines execute, not that they execute correctly. Combine coverage with invariant testing and fuzzing.
Continuous Integration Pattern
A baseline CI configuration that integrates these tools:
# .github/workflows/security.yml (excerpt)
name: Security Checks
on: [push, pull_request]
jobs:
security:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: foundry-rs/foundry-toolchain@v1
# Linting
- run: npm install -g solhint
- run: solhint 'src/**/*.sol'
# Build with warnings as errors
- run: forge build --deny-warnings
# Tests with coverage
- run: forge test
- run: forge coverage --report summary
# Static analysis
- uses: crytic/slither-action@v0.4.0
with:
fail-on: high
# Verify storage layout (for upgradeable contracts)
- run: forge inspect MyContract storage-layout > storage.json
- run: git diff --exit-code storage.json
Every commit runs the full security suite. Pull requests that fail any check don't merge. The development team feels the security checks continuously rather than facing a large remediation task before each external audit.
Threat Modeling
Threat modeling is the practice of systematically asking what could go wrong. Unlike code review (which finds bugs in existing code), threat modeling identifies risks in the design — bugs that haven't been written yet because the design itself is flawed.
A practical threat modeling exercise for a smart contract follows a four-step process.
Step 1: Identify the Assets
Enumerate everything of value the contract holds, controls, or affects:
- ETH balance held by the contract
- ERC-20 tokens held by the contract
- NFTs held by the contract
- Privileged roles (owner, admin, operator)
- External integrations (oracle reads, other protocols' state)
- User account balances or positions tracked in the contract
Each asset is a target for attackers. Each operation that modifies an asset is a potential attack surface.
Step 2: Identify the Trust Boundaries
For each operation, identify what the contract trusts to behave correctly:
- The caller (msg.sender) — what privileges do they need?
- External contracts being called — what do we assume about their behavior?
- Oracle data — what do we assume about its accuracy and freshness?
- Block data (timestamp, hash) — what assumptions do we make about miner/proposer behavior?
- Input parameters — what range of values do we expect?
Each assumption is a trust boundary. Each trust boundary is a potential attack surface — if the assumption is wrong, the operation fails.
Step 3: Enumerate Attack Scenarios
For each asset and each trust boundary, ask: how could this be attacked? The cataloged vulnerabilities in Section 3.8 are the starting list:
- Reentrancy — could an external call re-enter the function?
- Front-running — could a searcher reorder around this transaction?
- Oracle manipulation — could an attacker manipulate the oracle reading?
- Arithmetic — could the inputs cause precision loss or overflow?
- Access control — could an unauthorized party call this?
- DoS — could an attacker make this function uncallable?
- Signature/replay — could a signature be reused or forged?
For each, the answer should be one of: no, because <defense>; yes, but the impact is bounded by <defense>; or yes, this is a problem we need to address.
Step 4: Document Invariants
Invariants are properties of the system that must hold no matter what sequence of operations occurs. They are the testable statements of "the contract is functioning correctly."
Examples for a lending protocol:
- Solvency:
sum(user_collateral_value * price) >= sum(user_debt * 1.5)for every user - Conservation of supply:
total_supply == sum(balances) + locked_for_repayment - Borrowing limits:
user_debt[user] <= user_max_borrow[user]at all times - Liquidation profitability: A successful liquidation always reduces total_debt by at least the liquidated_amount
Invariants documented at threat-modeling time become the basis for invariant testing (Foundry's forge test with invariant tests, Echidna's stateful fuzzing). The act of writing them down forces clarity about what the system is supposed to guarantee.
Threat Model Document
The output of threat modeling is a document. A reasonable template:
# Threat Model: <Contract Name>
## Assets
- [Asset 1]: [What it is, where it's held, how it's protected]
- [Asset 2]: ...
## Trust Boundaries
- [Boundary 1]: [What we trust, what could go wrong if trust is violated]
- ...
## Attack Scenarios
| Asset | Attack | Defense | Residual Risk |
|---|---|---|---|
| ETH balance | Reentrancy on withdraw | nonReentrant + CEI | None |
| ETH balance | Oracle manipulation via flash loan | Chainlink TWAP with 30-min window | Low — sustained manipulation costs > available value |
| ...
## Invariants
1. [Invariant 1] — testable expression
2. [Invariant 2] — ...
This document is the auditor's most valuable single input. It tells them what the contract is supposed to do, what risks the team has identified, and what assumptions the team is making. An auditor's job becomes "verify these claims" rather than "construct this analysis from scratch."
Test Coverage as Internal Audit
The line between "good tests" and "internal audit" is blurry. Sufficient test coverage is internal audit. The patterns:
Test Every Reverting Branch
For every require, every revert, every assert, there should be a test that triggers that exact failure. These tests are easy to write and routinely skipped — most developers write tests for the happy path and stop. The negative tests are the ones that catch bugs.
// For every revert, a test
function test_withdrawRevertsOnInsufficientBalance() public {
vm.expectRevert("insufficient balance");
vault.withdraw(1 ether);
}
function test_setFeeRateRevertsAboveCap() public {
vm.prank(operator);
vm.expectRevert("fee too high");
protocol.setFeeRate(1001);
}
Test Every Access Boundary
For every privileged function, test that unauthorized callers cannot call it. The pattern from Section 3.8.4:
function test_unprivilegedCallerCannotPause() public {
address attacker = makeAddr("attacker");
vm.prank(attacker);
vm.expectRevert();
protocol.pause();
}
Test Composability with Adversarial Contracts
For every function that makes an external call, test that a malicious recipient cannot exploit the call. Write attacker contracts that revert, that consume all gas, that re-enter, that return malformed data — and verify the calling contract handles each correctly.
Invariant Testing
Foundry's invariant testing fuzzes the contract with arbitrary call sequences and verifies that properties hold across all reachable states. The invariants identified in threat modeling become the assertions:
contract LendingInvariantTest is Test {
LendingProtocol protocol;
function setUp() public { /* ... */ }
function invariant_solvency() public view {
uint256 totalCollateralValue = 0;
uint256 totalDebt = 0;
for (uint256 i = 0; i < users.length; ++i) {
totalCollateralValue += protocol.collateralValue(users[i]);
totalDebt += protocol.debt(users[i]);
}
assertGe(totalCollateralValue, totalDebt * 150 / 100, "solvency invariant violated");
}
}
Section 4.8 covers fuzzing and invariant testing in depth. The pattern's value for internal audit is that it surfaces edge cases the developer would never construct manually — and surfaces them before an external auditor would.
The Internal Audit Cycle
Putting the four pillars together as a workflow:
- Every commit: solhint runs, Slither runs, tests run, coverage is checked, warnings are errors
- Every PR: human peer review with the checklist; reviewers explicitly verify each item
- Every release candidate: full Mythril analysis; threat model document is updated to reflect new functionality; invariant tests are extended
- Before any external audit: a structured internal audit pass — at minimum a senior developer's full read-through of the changed code with the threat model and invariants in hand, plus a final tooling sweep
The internal audit cycle is not a one-shot event. It is the development team's continuous discipline. External audits punctuate the process; they don't replace it.
What Internal Audits Catch (and What They Don't)
A realistic view:
Internal audits routinely catch:
- Missing access control modifiers
- CEI violations and basic reentrancy
- Unchecked external calls
- Variable shadowing and naming bugs
- Missing event emissions
- Test coverage gaps
- Pattern violations (using
tx.origin, hardcoded addresses, magic numbers)
Internal audits often miss:
- Subtle business-logic bugs that depend on multi-step interactions
- Economic attack vectors (flash loans, oracle manipulation in unusual configurations)
- Cross-contract interactions with third-party protocols
- Timing-sensitive vulnerabilities (front-running variants, MEV exposure)
- Cryptographic subtleties (signature malleability, EIP-712 mistakes)
- Bugs that require specific market conditions to trigger
The bugs internal review misses are exactly the bugs external auditors are expert at finding. The complementary relationship is what makes the combination effective: internal review handles volume and breadth; external audit handles depth and subtlety.
Cross-References
- External audit preparation — Section 3.9.2 covers what to deliver to the auditor once internal review is complete
- Selecting an audit path — Section 3.9.3 covers when each audit format is appropriate
- Patterns and anti-patterns — Sections 3.7 and 3.8 catalog what the reviewer is looking for during internal audit
- Static analysis tools — Section 4.6 covers Slither and similar tooling from the auditor's perspective
- Fuzzing and invariant testing — Section 4.8 covers stateful fuzzing with Echidna and Foundry
- Threat modeling at the SDLC level — Section 2.1.2 covers threat modeling earlier in the design phase
- OpenZeppelin Contracts Wizard — for new projects, starting from generated OZ scaffolding reduces the surface area of code that needs internal review (https://wizard.openzeppelin.com)