From e23b3d51b155bfd6f18f63f282eea03923d69c3d Mon Sep 17 00:00:00 2001 From: Nathaniel Nanle Date: Wed, 29 Jul 2026 11:31:25 +0100 Subject: [PATCH] feat: detect vault share-price inflation attacks --- README.md | 1 + .../contracts/ProtectedInflationVault.sol | 54 +++ examples/contracts/UnrelatedRatioMath.sol | 17 + .../contracts/VulnerableInflationVault.sol | 45 +++ packages/core/src/index.ts | 2 +- .../__tests__/cp122-vault-inflation.test.ts | 52 +++ .../core/src/rules/cp122-vault-inflation.ts | 351 ++++++++++++++++++ packages/core/src/scanner.ts | 3 + packages/server/src/rules-registry.ts | 10 + 9 files changed, 534 insertions(+), 1 deletion(-) create mode 100644 examples/contracts/ProtectedInflationVault.sol create mode 100644 examples/contracts/UnrelatedRatioMath.sol create mode 100644 examples/contracts/VulnerableInflationVault.sol create mode 100644 packages/core/src/rules/__tests__/cp122-vault-inflation.test.ts create mode 100644 packages/core/src/rules/cp122-vault-inflation.ts diff --git a/README.md b/README.md index 37a262e..26389b4 100644 --- a/README.md +++ b/README.md @@ -361,6 +361,7 @@ See [`.github/workflows/audit.yml`](.github/workflows/audit.yml) for a complete | CP-115 | [SWC-115](https://swcregistry.io/docs/SWC-115) | `tx.origin` authentication | High | `tx.origin` used in `require` or access control | | CP-101 | [SWC-101](https://swcregistry.io/docs/SWC-101) | Integer overflow / underflow | High | Arithmetic on pragma `< 0.8` without SafeMath | | CP-104 | [SWC-104](https://swcregistry.io/docs/SWC-104) | Unchecked call return value | Medium | `.call` / `.send` return value not checked | +| CP-122 | — | Vault share-price inflation | High | Live-balance share ratio without initialization protection | | GAS-\* | — | Gas optimizations | Gas | Storage in loops, packing, `keccak256`, etc. | When Slither is installed, all [Slither detectors](https://github.com/crytic/slither/wiki/Detector-Documentation) are merged in with deduplication by line + title. Slither findings are prefixed with `SLITHER-`. diff --git a/examples/contracts/ProtectedInflationVault.sol b/examples/contracts/ProtectedInflationVault.sol new file mode 100644 index 0000000..276abfc --- /dev/null +++ b/examples/contracts/ProtectedInflationVault.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +interface IProtectedVaultAsset { + function balanceOf(address account) external view returns (uint256); + function transfer(address to, uint256 amount) external returns (bool); + function transferFrom(address from, address to, uint256 amount) external returns (bool); +} + +/// @notice Dead-share initialization prevents a first depositor owning all supply. +contract ProtectedInflationVault { + uint256 private constant MINIMUM_LIQUIDITY = 1_000; + address private constant DEAD_SHARES_RECEIVER = address(0xdead); + + IProtectedVaultAsset public immutable asset; + uint256 public totalSupply; + mapping(address => uint256) public balanceOf; + + constructor(IProtectedVaultAsset asset_) { + asset = asset_; + } + + function totalAssets() public view returns (uint256) { + return asset.balanceOf(address(this)); + } + + function deposit(uint256 assets, address receiver) external returns (uint256 shares) { + uint256 supply = totalSupply; + uint256 assetsBefore = totalAssets(); + + if (supply == 0) { + require(assets > MINIMUM_LIQUIDITY, "MINIMUM_DEPOSIT"); + _mint(DEAD_SHARES_RECEIVER, MINIMUM_LIQUIDITY); + shares = assets - MINIMUM_LIQUIDITY; + } else { + shares = assets * totalSupply / assetsBefore; + } + + require(asset.transferFrom(msg.sender, address(this), assets), "TRANSFER_FAILED"); + _mint(receiver, shares); + } + + function withdraw(uint256 assets, address receiver) external returns (uint256 shares) { + shares = assets * totalSupply / totalAssets(); + balanceOf[msg.sender] -= shares; + totalSupply -= shares; + require(asset.transfer(receiver, assets), "TRANSFER_FAILED"); + } + + function _mint(address receiver, uint256 shares) internal { + balanceOf[receiver] += shares; + totalSupply += shares; + } +} diff --git a/examples/contracts/UnrelatedRatioMath.sol b/examples/contracts/UnrelatedRatioMath.sol new file mode 100644 index 0000000..ccf6752 --- /dev/null +++ b/examples/contracts/UnrelatedRatioMath.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +/// @notice Similar proportional arithmetic, but this is not a share vault. +contract UnrelatedRatioMath { + uint256 public totalSupply; + uint256 public totalAssets; + + function quote(uint256 amount) external view returns (uint256) { + return amount * totalSupply / totalAssets; + } + + function updateTotals(uint256 supply, uint256 assets) external { + totalSupply = supply; + totalAssets = assets; + } +} diff --git a/examples/contracts/VulnerableInflationVault.sol b/examples/contracts/VulnerableInflationVault.sol new file mode 100644 index 0000000..f056290 --- /dev/null +++ b/examples/contracts/VulnerableInflationVault.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +interface IInflationVaultAsset { + function balanceOf(address account) external view returns (uint256); + function transfer(address to, uint256 amount) external returns (bool); + function transferFrom(address from, address to, uint256 amount) external returns (bool); +} + +/// @notice Deliberately vulnerable example for CP-122. +contract VulnerableInflationVault { + IInflationVaultAsset public immutable asset; + uint256 public totalSupply; + mapping(address => uint256) public balanceOf; + + constructor(IInflationVaultAsset asset_) { + asset = asset_; + } + + function totalAssets() public view returns (uint256) { + // Direct transfers change this value without minting any shares. + return asset.balanceOf(address(this)); + } + + function deposit(uint256 assets, address receiver) external returns (uint256 shares) { + uint256 supply = totalSupply; + shares = supply == 0 ? assets : assets * totalSupply / totalAssets(); + require(shares != 0, "ZERO_SHARES"); + + require(asset.transferFrom(msg.sender, address(this), assets), "TRANSFER_FAILED"); + _mint(receiver, shares); + } + + function withdraw(uint256 assets, address receiver) external returns (uint256 shares) { + shares = assets * totalSupply / totalAssets(); + balanceOf[msg.sender] -= shares; + totalSupply -= shares; + require(asset.transfer(receiver, assets), "TRANSFER_FAILED"); + } + + function _mint(address receiver, uint256 shares) internal { + balanceOf[receiver] += shares; + totalSupply += shares; + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 345af89..eecec04 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -28,6 +28,7 @@ export { checkERC721Compliance, checkERC1155Compliance, } from "./rules/erc-compliance"; +export { detectVaultInflation } from "./rules/cp122-vault-inflation"; export { generateMarkdownReport, generateJSONReport, @@ -38,7 +39,6 @@ export { } from "./report/generator"; export { diffScans, computeFingerprint } from "./diff"; export { isSlitherAvailable } from "./ast/slither"; -export { enhanceFindingsWithLLM } from "./llm/enhancer"; export { loadPlugin, loadPlugins } from "./plugins"; export { loadConfigFile, diff --git a/packages/core/src/rules/__tests__/cp122-vault-inflation.test.ts b/packages/core/src/rules/__tests__/cp122-vault-inflation.test.ts new file mode 100644 index 0000000..0dcf3c3 --- /dev/null +++ b/packages/core/src/rules/__tests__/cp122-vault-inflation.test.ts @@ -0,0 +1,52 @@ +import * as fs from "fs"; +import * as path from "path"; +import { parseSolidity } from "../../ast/parser"; +import { scan } from "../../scanner"; +import { detectVaultInflation } from "../cp122-vault-inflation"; + +const FIXTURES_DIR = path.resolve(__dirname, "../../../../../examples/contracts"); + +function detectInFixture(fileName: string) { + const file = path.join(FIXTURES_DIR, fileName); + const source = fs.readFileSync(file, "utf-8"); + const { ast, error } = parseSolidity(source, file); + expect(error).toBeUndefined(); + expect(ast).not.toBeNull(); + return detectVaultInflation(ast!, source, file); +} + +describe("CP-122 vault share-price inflation", () => { + it("flags a live-balance vault with naive first-depositor share math", () => { + const findings = detectInFixture("VulnerableInflationVault.sol"); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + id: "CP-122", + severity: "high", + }); + expect(findings[0].recommendation).toContain("dead shares"); + expect(findings[0].recommendation).toContain("internal accounting"); + expect(findings[0].snippet).toContain("assets * totalSupply / totalAssets()"); + }); + + it("does not flag a vault protected by minimum-liquidity dead shares", () => { + expect(detectInFixture("ProtectedInflationVault.sol")).toEqual([]); + }); + + it("does not flag unrelated proportional arithmetic", () => { + expect(detectInFixture("UnrelatedRatioMath.sol")).toEqual([]); + }); + + it("is enabled in the built-in scanner", async () => { + const file = path.join(FIXTURES_DIR, "VulnerableInflationVault.sol"); + const result = await scan({ + targets: [file], + useSlither: false, + useLLM: false, + useMetrics: false, + }); + + const finding = result.files[0].findings.find((item) => item.id === "CP-122"); + expect(finding?.severity).toBe("high"); + }); +}); diff --git a/packages/core/src/rules/cp122-vault-inflation.ts b/packages/core/src/rules/cp122-vault-inflation.ts new file mode 100644 index 0000000..6df7ed2 --- /dev/null +++ b/packages/core/src/rules/cp122-vault-inflation.ts @@ -0,0 +1,351 @@ +import { getSnippet, visit } from "../ast/parser"; +import type { MergedMember } from "../ast/import-graph"; +import type { ASTNode, Finding } from "../types"; +import { applyFindingContext, type RuleOptions } from "./rule-context"; + +/** + * CP-122: ERC-4626 / vault share-price inflation. + * + * This detector intentionally requires several independent vault signals before + * reporting a finding: + * + * 1. deposit/mint and withdraw/redeem entry points; + * 2. a share-minting operation; + * 3. assets * totalSupply / totalAssets-style conversion math; and + * 4. total-assets accounting backed by the asset token's live balance. + * + * The finding is suppressed when dead/locked shares, virtual assets and shares, + * a decimals offset, or an explicit minimum-liquidity initialization is found. + * Requiring the full conjunction avoids treating unrelated proportional math as + * vulnerable vault accounting. + */ + +interface FunctionInfo { + name: string; + node: ASTNode; + source: string; + member?: MergedMember; +} + +interface ContractAnalysis { + name: string; + functions: FunctionInfo[]; + stateVariableNames: string[]; +} + +interface RatioMatch { + node: ASTNode; + fn: FunctionInfo; + hasTwoSidedOffset: boolean; +} + +const ENTRY_FUNCTIONS = new Set(["deposit", "mint"]); +const EXIT_FUNCTIONS = new Set(["withdraw", "redeem"]); +const CONVERSION_FUNCTION = /^(previewdeposit|previewmint|converttoshares|_converttoshares|deposit|mint)$/; +const PROTECTION_NAME = + /(virtual|decimalsoffset|minimum(deposit|liquidity)|deadshares?|lockedshares?|seedshares?|initialshares?)/; + +export function detectVaultInflation( + ast: ASTNode, + source: string, + filePath: string, + options?: RuleOptions, +): Finding[] { + const analyses = options?.contractView + ? [analysisFromView(options.contractView.name, options.contractView.members)] + : analysesFromAst(ast, source); + + const findings: Finding[] = []; + for (const analysis of analyses) { + const finding = analyzeContract(analysis, filePath, options); + if (finding) findings.push(finding); + } + return findings; +} + +function analysesFromAst(ast: ASTNode, source: string): ContractAnalysis[] { + const contracts: ContractAnalysis[] = []; + + visit(ast, { + ContractDefinition(node: ASTNode) { + const contract = node as { name?: string }; + const functions: FunctionInfo[] = []; + const stateVariableNames: string[] = []; + + visit(node, { + FunctionDefinition(fnNode: ASTNode) { + const fn = fnNode as { name?: string; isConstructor?: boolean }; + if (!fn.isConstructor && fn.name) { + functions.push({ name: fn.name, node: fnNode, source }); + } + }, + StateVariableDeclaration(declarationNode: ASTNode) { + const declaration = declarationNode as { + variables?: Array<{ name?: string }>; + }; + for (const variable of declaration.variables ?? []) { + if (variable.name) stateVariableNames.push(variable.name); + } + }, + }); + + contracts.push({ + name: contract.name ?? "vault", + functions, + stateVariableNames, + }); + }, + }); + + return contracts; +} + +function analysisFromView( + contractName: string, + members: MergedMember[], +): ContractAnalysis { + return { + name: contractName, + functions: members + .filter((member) => member.kind === "function") + .map((member) => ({ + name: member.name, + node: member.node, + source: member.source, + member, + })), + stateVariableNames: members + .filter((member) => member.kind === "stateVariable") + .map((member) => member.name), + }; +} + +function analyzeContract( + contract: ContractAnalysis, + filePath: string, + options?: RuleOptions, +): Finding | null { + const functionNames = new Set(contract.functions.map((fn) => fn.name.toLowerCase())); + const hasEntry = [...ENTRY_FUNCTIONS].some((name) => functionNames.has(name)); + const hasExit = [...EXIT_FUNCTIONS].some((name) => functionNames.has(name)); + if (!hasEntry || !hasExit || !mintsShares(contract.functions)) return null; + + const ratio = findNaiveShareRatio(contract.functions); + if (!ratio) return null; + + const usesLiveAssetBalance = + expressionUsesLiveBalance(ratio.node) || + contract.functions.some( + (fn) => + normalizeName(fn.name) === "totalassets" && + nodeUsesLiveBalance(fn.node), + ); + if (!usesLiveAssetBalance || hasInflationProtection(contract, ratio)) { + return null; + } + + const line = + (ratio.node as { loc?: { start?: { line?: number } } }).loc?.start?.line ?? + 1; + + return applyFindingContext( + { + id: "CP-122", + title: `Vault share-price inflation risk in ${contract.name}`, + description: + `"${contract.name}" mints shares using a total-supply/total-assets ratio while ` + + "total assets come from the token's live balance. An attacker can mint the first " + + "shares with a minimal deposit, donate tokens directly to the vault, and cause a " + + "later depositor's share amount to round down to zero or a negligible value.", + recommendation: + `Protect ${contract.name}'s ${ratio.fn.name} share conversion by adding virtual ` + + "assets and virtual shares (with a decimals offset), or lock a fixed minimum " + + "number of dead shares during first-deposit initialization. For stronger donation " + + "resistance, track managed assets in an internal accounting variable instead of " + + "using the token's live balanceOf(address(this)).", + severity: "high", + file: filePath, + line, + snippet: getSnippet(ratio.fn.source, ratio.node), + }, + ratio.fn.member, + options?.contractView, + ); +} + +function mintsShares(functions: FunctionInfo[]): boolean { + return functions.some((fn) => { + let found = false; + visit(fn.node, { + FunctionCall(node: ASTNode) { + const expression = (node as { expression?: ASTNode }).expression; + const calledName = getCalledName(expression); + if (calledName === "_mint" || calledName === "mint") found = true; + }, + }); + return found; + }); +} + +function findNaiveShareRatio(functions: FunctionInfo[]): RatioMatch | null { + for (const fn of functions) { + if (!CONVERSION_FUNCTION.test(normalizeName(fn.name))) continue; + + let match: RatioMatch | null = null; + const parameterNames = new Set( + ( + fn.node as { + parameters?: Array<{ name?: string }>; + } + ).parameters + ?.map((parameter) => parameter.name?.toLowerCase()) + .filter((name): name is string => Boolean(name)) ?? [], + ); + + visit(fn.node, { + BinaryOperation(node: ASTNode) { + if (match) return; + const operation = node as { + operator?: string; + left?: ASTNode; + right?: ASTNode; + }; + if (operation.operator !== "/" || !operation.left || !operation.right) { + return; + } + + const numeratorNames = collectNames(operation.left); + const denominatorNames = collectNames(operation.right); + const numeratorHasProduct = containsOperator(operation.left, "*"); + const usesAssetInput = [...numeratorNames].some( + (name) => + parameterNames.has(name) && + /(asset|amount|deposit|value)/.test(name), + ); + const usesSupply = [...numeratorNames].some((name) => + /totalsupply|sharesupply/.test(name), + ); + const usesAssets = [...denominatorNames].some((name) => + /totalassets|assetbalance|balanceof/.test(name), + ); + + if (numeratorHasProduct && usesAssetInput && usesSupply && usesAssets) { + match = { + node, + fn, + hasTwoSidedOffset: + containsOperator(operation.left, "+") && + containsOperator(operation.right, "+"), + }; + } + }, + }); + + if (match) return match; + } + return null; +} + +function hasInflationProtection( + contract: ContractAnalysis, + ratio: RatioMatch, +): boolean { + if (ratio.hasTwoSidedOffset) return true; + + const names = [ + ...contract.stateVariableNames, + ...contract.functions.map((fn) => fn.name), + ]; + if (names.some((name) => PROTECTION_NAME.test(normalizeName(name)))) { + return true; + } + + return contract.functions.some((fn) => { + const compact = getSnippet(fn.source, fn.node) + .toLowerCase() + .replace(/\s+/g, ""); + return ( + /_?mint\(address\((0|0x0+|0xdead)\),/.test(compact) || + /_?mint\(0x0+,/.test(compact) + ); + }); +} + +function nodeUsesLiveBalance(node: ASTNode): boolean { + let usesBalanceOf = false; + let usesThisAddress = false; + + visit(node, { + MemberAccess(child: ASTNode) { + const access = child as { memberName?: string }; + if (access.memberName?.toLowerCase() === "balanceof") { + usesBalanceOf = true; + } + }, + Identifier(child: ASTNode) { + const identifier = child as { name?: string }; + if (identifier.name?.toLowerCase() === "this") usesThisAddress = true; + }, + }); + + // Some parser versions represent `this` as a primary expression rather than + // an Identifier, so retain a source-independent structural balanceOf signal. + return usesBalanceOf && (usesThisAddress || containsAddressThisCall(node)); +} + +function expressionUsesLiveBalance(node: ASTNode): boolean { + return nodeUsesLiveBalance(node); +} + +function containsAddressThisCall(node: ASTNode): boolean { + let found = false; + visit(node, { + FunctionCall(child: ASTNode) { + const call = child as { expression?: ASTNode; arguments?: ASTNode[] }; + if ( + getCalledName(call.expression)?.toLowerCase() === "address" && + (call.arguments ?? []).some((argument) => + collectNames(argument).has("this"), + ) + ) { + found = true; + } + }, + }); + return found; +} + +function collectNames(node: ASTNode): Set { + const names = new Set(); + visit(node, { + Identifier(child: ASTNode) { + const identifier = child as { name?: string }; + if (identifier.name) names.add(identifier.name.toLowerCase()); + }, + MemberAccess(child: ASTNode) { + const access = child as { memberName?: string }; + if (access.memberName) names.add(access.memberName.toLowerCase()); + }, + }); + return names; +} + +function containsOperator(node: ASTNode, operator: string): boolean { + let found = false; + visit(node, { + BinaryOperation(child: ASTNode) { + if ((child as { operator?: string }).operator === operator) found = true; + }, + }); + return found; +} + +function getCalledName(expression?: ASTNode): string | undefined { + if (!expression) return undefined; + const value = expression as { name?: string; memberName?: string }; + return value.name ?? value.memberName; +} + +function normalizeName(name: string): string { + return name.toLowerCase().replace(/_/g, ""); +} diff --git a/packages/core/src/scanner.ts b/packages/core/src/scanner.ts index 8bb2261..a0a0bfe 100644 --- a/packages/core/src/scanner.ts +++ b/packages/core/src/scanner.ts @@ -23,6 +23,7 @@ import { checkERC721Compliance, checkERC1155Compliance, } from "./rules/erc-compliance"; +import { detectVaultInflation } from "./rules/cp122-vault-inflation"; import { RuleOptions } from "./rules/rule-context"; import { detectGasIssues } from "./rules/gas-optimizer"; import { enhanceFindingsWithLLM } from "./llm/enhancer"; @@ -94,6 +95,7 @@ function runRulesOnView( ...detectIntegerOverflow(view.node, view.source, view.file), ...detectUncheckedReturn(view.node, view.source, view.file), ...runERCChecks(view.node, view.source, view.file, ruleOptions), + ...detectVaultInflation(view.node, view.source, view.file, ruleOptions), ]; } @@ -109,6 +111,7 @@ function runRulesOnFile( ...detectIntegerOverflow(ast, source, filePath), ...detectUncheckedReturn(ast, source, filePath), ...runERCChecks(ast, source, filePath), + ...detectVaultInflation(ast, source, filePath), ]; } diff --git a/packages/server/src/rules-registry.ts b/packages/server/src/rules-registry.ts index 495bf56..55442e1 100644 --- a/packages/server/src/rules-registry.ts +++ b/packages/server/src/rules-registry.ts @@ -58,6 +58,16 @@ export const RULES: RuleMeta[] = [ "Ignoring this leaves the contract in an inconsistent state on failure. " + "Always check the return value or use transfer().", }, + { + id: "CP-122", + title: "Vault share-price inflation", + severity: "high", + category: "security", + description: + "Detects ERC-4626-style vaults that combine naive share-ratio math with live " + + "token-balance accounting and no virtual offset, dead shares, or minimum-liquidity " + + "initialization, exposing later depositors to first-depositor donation attacks.", + }, { id: "GAS-LOOP-STORAGE", title: "Storage read inside loop",