diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..37fb8c6 --- /dev/null +++ b/Makefile @@ -0,0 +1,235 @@ +# .d8888b. +# d88P Y88b +# Y88b. +# "Y888b. 888 888 888 8888b. 888d888 88888b.d88b. +# "Y88b. 888 888 888 "88b 888P" 888 "888 "88b +# "888 888 888 888 .d888888 888 888 888 888 +# Y88b d88P Y88b 888 d88P 888 888 888 888 888 888 +# "Y8888P" "Y8888888P" "Y888888 888 888 888 888 +# +# +# +# .d8888b. d8b 888 +# d88P Y88b Y8P 888 +# Y88b. 888 +# "Y888b. .d88b. .d8888b 888 888 888d888 888 888888 888 888 +# "Y88b. d8P Y8b d88P" 888 888 888P" 888 888 888 888 +# "888 88888888 888 888 888 888 888 888 888 888 +# Y88b d88P Y8b. Y88b. Y88b 888 888 888 Y88b. Y88b 888 +# "Y8888P" "Y8888 "Y8888P "Y88888 888 888 "Y888 "Y88888 +# 888 +# Y8b d88P +# "Y88P" +# d8888 +# d88888 +# d88P888 +# d88P 888 888d888 .d88b. 888 888 .d88b. 888 888 +# d88P 888 888P" d8P Y8b 888 888 d88""88b 888 888 +# d88P 888 888 88888888 888 888 888 888 888 888 +# d8888888888 888 Y8b. Y88b 888 Y88..88P Y88b 888 +# d88P 888 888 "Y8888 "Y88888 "Y88P" "Y88888 +# 888 +# Y8b d88P +# "Y88P" +# 888 888 +# 888 888 +# 888 888 +# 888d888 .d88b. 8888b. .d88888 888 888 888888 .d88b. +# 888P" d8P Y8b "88b d88" 888 888 888 888 d88""88b +# 888 88888888 .d888888 888 888 888 888 888 888 888 +# 888 Y8b. 888 888 Y88b 888 Y88b 888 Y88b. Y88..88P +# 888 "Y8888 "Y888888 "Y88888 "Y88888 "Y888 "Y88P" +# 888 +# Y8b d88P +# "Y88P" +# 888 +# 888 +# 888 +# 8888b. 88888b. 8888b. 888 888 888 .d8888b .d88b. +# "88b 888 "88b "88b 888 888 888 88K d8P Y8b +# .d888888 888 888 .d888888 888 888 888 "Y8888b. 88888888 +# 888 888 888 888 888 888 888 Y88b 888 X88 Y8b. +# "Y888888 888 888 "Y888888 888 "Y88888 88888P' "Y8888 +# 888 +# Y8b d88P +# "Y88P" +# +# +# +# 888 888 .d88b. 888 888 888d888 +# 888 888 d88""88b 888 888 888P" +# 888 888 888 888 888 888 888 +# Y88b 888 Y88..88P Y88b 888 888 +# "Y88888 "Y88P" "Y88888 888 +# 888 +# Y8b d88P +# "Y88P" +# 888 888 +# 888 888 +# 888 888 +# .d8888b .d88b. 88888b. 888888 888d888 8888b. .d8888b 888888 .d8888b +# d88P" d88""88b 888 "88b 888 888P" "88b d88P" 888 88K +# 888 888 888 888 888 888 888 .d888888 888 888 "Y8888b. +# Y88b. Y88..88P 888 888 Y88b. 888 888 888 Y88b. Y88b. X88 +# "Y8888P "Y88P" 888 888 "Y888 888 "Y888888 "Y8888P "Y888 88888P' +# +# +# +# d8b 888 888 888 888 +# Y8P 888 888 888 888 +# 888 888 888 888 +# 888 888 888 888 888888 88888b. 888888 88888b. .d88b. +# 888 888 888 888 888 888 "88b 888 888 "88b d8P Y8b +# 888 888 888 888 888 888 888 888 888 888 88888888 +# Y88b 888 d88P 888 Y88b. 888 888 Y88b. 888 888 Y8b. +# "Y8888888P" 888 "Y888 888 888 "Y888 888 888 "Y8888 +# +# +# +# 888 +# 888 +# 888 +# .d88b. 888 888 88888b. .d88b. 888d888 888888 +# d8P Y8b `Y8bd8P' 888 "88b d8P Y8b 888P" 888 +# 88888888 X88K 888 888 88888888 888 888 +# Y8b. .d8""8b. 888 d88P Y8b. 888 Y88b. +# "Y8888 888 888 88888P" "Y8888 888 "Y888 +# 888 +# 888 +# 888 +# 888 +# 888 +# 888 +# 8888b. .d88b. .d88b. 88888b. 888888 .d8888b +# "88b d88P"88b d8P Y8b 888 "88b 888 88K +# .d888888 888 888 88888888 888 888 888 "Y8888b. +# 888 888 Y88b 888 Y8b. 888 888 Y88b. X88 d8b +# "Y888888 "Y88888 "Y8888 888 888 "Y888 88888P' Y8P +# 888 +# Y8b d88P +# "Y88P" + +# RedSpectre Makefile + +.PHONY: help setup-env setup-venv install install-deps server tunnel setup all + +# Default target +help: + @echo "RedSpectre Makefile" + @echo "" + @echo "Available targets:" + @echo " setup-env - Create .env file by prompting for required keys" + @echo " setup-venv - Create Python virtual environment" + @echo " install - Install Python packages (requires venv to be activated)" + @echo " install-deps - Install npm global dependencies (localtunnel)" + @echo " server - Start audit-agent server on port 8000" + @echo " tunnel - Start localtunnel on port 8000 (requires server to be running)" + @echo " setup - Run full setup: env, venv, install, install-deps" + @echo " all - Run setup, start server, and start tunnel" + +# Create .env file by prompting for required keys +setup-env: + @if [ -f .env ]; then \ + echo ".env file already exists. Remove it first if you want to recreate it."; \ + exit 1; \ + fi + @echo "Creating .env file..." + @bash -c '\ + echo "# OpenAI Coiguration" > .env; \ + read -sp "Enter OPENAI_API_KEY: " key; echo ""; { echo "OPENAI_API_KEY=$$key"; } >> .env; \ + read -p "Enter OPENAI_MODEL [gpt-4.1-nano-2025-04-14]: " model && echo "OPENAI_MODEL=$${model:-gpt-4.1-nano-2025-04-14}" >> .env || echo "OPENAI_MODEL=gpt-4.1-nano-2025-04-14" >> .env; \ + echo "" >> .env; \ + echo "# Logging" >> .env; \ + read -p "Enter LOG_LEVEL [INFO]: " level && echo "LOG_LEVEL=$${level:-INFO}" >> .env || echo "LOG_LEVEL=INFO" >> .env; \ + read -p "Enter LOG_FILE [agent.log]: " file && echo "LOG_FILE=$${file:-agent.log}" >> .env || echo "LOG_FILE=agent.log" >> .env; \ + echo "" >> .env; \ + echo "# Server Mode (Optional)" >> .env; \ + read -sp "Enter AGENTARENA_API_KEY (optional, press Enter to skip): " api_key; echo ""; { echo "AGENTARENA_API_KEY=$$api_key"; } >> .env; \ + read -sp "Enter WEBHOOK_AUTH_TOKEN (optional, press Enter to skip): " token; echo ""; { echo "WEBHOOK_AUTH_TOKEN=$$token"; } >> .env; \ + read -p "Enter DATA_DIR [./data]: " dir && echo "DATA_DIR=$${dir:-./data}" >> .env || echo "DATA_DIR=./data" >> .env; \ + echo ".env file created successfully!"' + +# Create virtual environment +setup-venv: + @if [ -d venv ]; then \ + echo "Virtual environment already exists. Remove it first if you want to recreate it."; \ + exit 1; \ + fi + @echo "Creating virtual environment..." + python3 -m venv venv + @echo "Virtual environment created. Activate it with: source venv/bin/activate" + +# Install Python packages (requires venv to be activated) +install: + @if [ -z "$$VIRTUAL_ENV" ]; then \ + echo "Error: Virtual environment not activated. Please run: source venv/bin/activate"; \ + exit 1; \ + fi + @echo "Installing Python packages..." + pip install -e . + @echo "Python packages installed successfully!" + +# Install npm global dependencies +install-deps: + @echo "Installing localtunnel globally..." + npm install -g localtunnel + @echo "localtunnel installed successfully!" + +# Start audit-agent server on port 8000 +server: + @if [ -z "$$VIRTUAL_ENV" ]; then \ + echo "Error: Virtual environment not activated. Please run: source venv/bin/activate"; \ + exit 1; \ + fi + @echo "Starting audit-agent server on port 8000..." + @audit-agent server + +# Start localtunnel on port 8000 (requires server to be running) +tunnel: + @if [ -z "$$VIRTUAL_ENV" ]; then \ + echo "Error: Virtual environment not activated. Please run: source venv/bin/activate"; \ + exit 1; \ + fi + @echo "Starting audit-agent server in background..." + @bash -c "source venv/bin/activate && audit-agent server &" + @sleep 3 + @echo "Server started. Starting localtunnel on port 8000..." + @echo "Add /webhook at the end of the tunnel address (e.g., https://siteaddress/webhook)" + @lt --port 8000 + +# Full setup: create env, venv, install packages, install deps +setup: setup-env setup-venv + @echo "" + @echo "==========================================" + @echo "Setup completed!" + @echo "==========================================" + @echo "" + @echo "Next steps:" + @echo "1. Activate the virtual environment:" + @echo " source venv/bin/activate" + @echo "" + @echo "2. Install Python packages:" + @echo " make install" + @echo "" + @echo "3. Install npm dependencies:" + @echo " make install-deps" + @echo "" + @echo "4. Start the server and tunnel:" + @echo " make tunnel" + @echo "" + +# Complete setup and start server + tunnel +all: setup + @echo "" + @echo "Activating virtual environment and installing packages..." + @bash -c "source venv/bin/activate && pip install -e ." + @echo "" + @echo "Installing npm dependencies..." + @npm install -g localtunnel || true + @echo "" + @echo "Setup complete! Starting server..." + @bash -c "source venv/bin/activate && audit-agent server &" + @sleep 3 + @echo "Server started. Starting tunnel..." + @echo "Add /webhook at the end of the tunnel address (e.g., https://siteaddress/webhook)" + @lt --port 8000 diff --git a/agent/services/auditor.py b/agent/services/auditor.py index 68163fa..4e43eda 100644 --- a/agent/services/auditor.py +++ b/agent/services/auditor.py @@ -4,8 +4,10 @@ """ import json import logging +import os +from collections import defaultdict from datetime import datetime -from typing import List +from typing import List, Dict, Any, Tuple from pydantic import BaseModel, Field from openai import OpenAI @@ -14,10 +16,51 @@ from agent.services.scout import Scout from agent.services.dedup import select_top_findings from agent.services.prompts.audit_prompt import AUDIT_PROMPT +from agent.services.reference_docs import DEFAULT_DOCS # -------------------------- logger = logging.getLogger(__name__) +def _load_slither_leads(report_path: str = "slither_report.json") -> Dict[str, List[Dict[str, Any]]]: + """Parse slither JSON report into a map of filename -> findings.""" + if not os.path.exists(report_path): + return {} + try: + with open(report_path) as f: + data = json.load(f) + except Exception: + return {} + leads = defaultdict(list) + for det in data.get("results", {}).get("detectors", []): + check = det.get("check") + impact = det.get("impact") + desc = det.get("description") or det.get("title") or "" + for elem in det.get("elements", []): + sm = elem.get("source_mapping", {}) or {} + filename = sm.get("filename") or sm.get("filename_absolute") or "" + lines = sm.get("lines") or [] + leads[filename].append({ + "check": check, + "impact": impact, + "description": elem.get("description") or desc, + "lines": lines, + }) + return leads + +def _match_leads(leads_map: Dict[str, List[Dict[str, Any]]], path: str) -> List[Dict[str, Any]]: + """Return slither leads for a given path, matching by full path or basename suffix.""" + if not leads_map: + return [] + if path in leads_map: + return leads_map[path] + basename = os.path.basename(path) + for k, v in leads_map.items(): + if basename and basename == os.path.basename(k): + return v + if path.endswith(k) or k.endswith(path): + return v + return [] + # Keep these models identical to the template so dependent files don't break class VulnerabilityFinding(BaseModel): """Model representing a single vulnerability finding.""" @@ -33,11 +76,11 @@ class Audit(BaseModel): class SolidityAuditor: """Service for auditing Solidity contracts using RedSpectre Swarm.""" - def __init__(self, api_key: str, model: str = "gpt-4o"): + def __init__(self, api_key: str, model: str = "gpt-4.1-mini"): """ Initialize the auditor with OpenAI credentials. """ - self.model = model + self.model = "gpt-4.1-mini" if (not model or model.lower().startswith("gpt-4o-mini")) else model self.client = OpenAI(api_key=api_key) # Initialize RedSpectre Components @@ -104,7 +147,7 @@ def _fallback_audit_prompt(self, contracts: List[object], docs: str, additional_ logger.warning("Fallback AUDIT_PROMPT failed: %s", e, exc_info=True) return [] - def audit_files(self, contracts: List[object], docs: str = "", additional_links: List[str] = None, additional_docs: str = None, qa_responses: List = None, benchmark_mode: bool = False) -> Audit: + def audit_files(self, contracts: List[object], docs: str = "", additional_links: List[str] = None, additional_docs: str = None, qa_responses: List = None, benchmark_mode: bool = True) -> Audit: """ RedSpectre Implementation: 1. Takes the list of SolidityFile objects (from local.py/server.py) @@ -115,6 +158,7 @@ def audit_files(self, contracts: List[object], docs: str = "", additional_links: logger.info("πŸš€ RedSpectre Swarm Activated") logger.info(f"Received contracts payload type={type(contracts)}") verified_findings = [] + slither_leads = {} # In the template, 'contracts' is passed as a List[SolidityFile] object in local.py # But sometimes as a string in other contexts. We handle the list case here. @@ -125,16 +169,37 @@ def audit_files(self, contracts: List[object], docs: str = "", additional_links: # 1. The Swarm Analysis Loop raw_persona_outputs = [] if benchmark_mode else None + # Trim prompt context: if user docs provided, use them; otherwise fallback to built-in refs. + enriched_docs = (docs or DEFAULT_DOCS).strip() + slither_summary_lines = [] + + slither_summary = "" + + persona_weights = { + "CentralizationExpert": 0.8, + "ValidationExpert": 0.8, + "InterfaceExpert": 0.8, + "EconomicExpert": 0.8, + "OracleExpert": 0.9, + "TimestampExpert": 0.9, + "TokenExpert": 0.9, + "FlashLoanExpert": 0.9, + "FrontrunningExpert": 0.9, + "Thief": 0.9, + "AccessControlExpert": 0.9, + } + for file_obj in files_to_audit: # file_obj has .path and .content attributes (from models/solidity_file.py) logger.info(f"Swarm analyzing: {file_obj.path}") + file_docs = enriched_docs # Call the Swarm # We pass the content, filename, and any supplemental context to the swarm logic swarm_results = self.swarm.analyze_file( file_obj.content, file_obj.path, - docs=docs, + docs=file_docs, additional_links=additional_links, additional_docs=additional_docs, qa_responses=qa_responses, @@ -143,24 +208,34 @@ def audit_files(self, contracts: List[object], docs: str = "", additional_links: logger.debug(f"Raw swarm results for {file_obj.path}: {swarm_results}") for res in swarm_results: + # Relaxed gates: require description, line, and severity >= Low + persona = res.get("detected_by", "unknown") + weight = persona_weights.get(persona, 1.0) + if not res.get("description") or res.get("line_number", 0) == 0: + continue + severity = res.get("severity", "High") + severity_rank = {"Critical": 4, "High": 3, "Medium": 2, "Low": 1, "Informational": 0} + if severity_rank.get(severity, 0) < 1: + continue + # Map RedSpectre result to AgentArena Finding Model - # We construct a detailed description including the reasoning logic detailed_desc = ( f"{res['description']}\n\n" - f"**Detected by:** {res['detected_by']} Persona\n" - f"**Attack Logic:** {res['attack_logic']}" + f"Attack Logic: {res.get('attack_logic') or 'Not provided'}\n" + f"Verification Proof: {res.get('verification_proof') or 'Not provided'}\n" + f"Detected by: {res['detected_by']} Persona" ) verified_findings.append(VulnerabilityFinding( title=res['title'], description=detailed_desc, - severity=res['severity'], + severity=severity, file_paths=[file_obj.path] )) if not verified_findings and files_to_audit: logger.info("No swarm findings; invoking fallback AUDIT_PROMPT.") - fallback = self._fallback_audit_prompt(files_to_audit, docs, additional_links or [], additional_docs, qa_responses or []) + fallback = self._fallback_audit_prompt(files_to_audit, enriched_docs, additional_links or [], additional_docs, qa_responses or []) for res in fallback: verified_findings.append(VulnerabilityFinding( title=res['title'], @@ -172,6 +247,9 @@ def audit_files(self, contracts: List[object], docs: str = "", additional_links: limited = select_top_findings(verified_findings, limit=20) if len(verified_findings) > 20: logger.info(f"Limiting findings to top {len(limited)} by consensus/severity (from {len(verified_findings)})") + # Ensure we never submit zero if any findings exist + if not limited and verified_findings: + limited = verified_findings[:20] logger.info(f"βœ… Audit completed with {len(limited)} returned findings (initial: {len(verified_findings)})") if benchmark_mode: diff --git a/agent/services/dedup.py b/agent/services/dedup.py index 0626d68..7d46734 100644 --- a/agent/services/dedup.py +++ b/agent/services/dedup.py @@ -79,10 +79,32 @@ def select_top_findings(findings: List[VulnerabilityFinding], limit: int = 20) - if "Consensus:" not in best.description: best.description = f"{best.description}{consensus_note}" - # Rank by consensus first, then severity, then number of affected files. + confidence = getattr(best, "confidence_score", None) + if confidence is None and isinstance(best, dict): + confidence = best.get("confidence_score") + if confidence is None: + confidence = 50 + try: + confidence = int(confidence) + except Exception: + confidence = 50 + + false_positive_risk = getattr(best, "false_positive_risk", None) + if false_positive_risk is None and isinstance(best, dict): + false_positive_risk = best.get("false_positive_risk") + if false_positive_risk is None: + false_positive_risk = 50 + try: + false_positive_risk = int(false_positive_risk) + except Exception: + false_positive_risk = 50 + + # Rank by consensus, severity, confidence, negative of false-positive risk, then breadth. score = ( support, severity_rank.get(best.severity, 0), + confidence, + -false_positive_risk, len(best.file_paths), ) scored.append((score, best)) diff --git a/agent/services/personas/audit_generalist.py b/agent/services/personas/audit_generalist.py new file mode 100644 index 0000000..5943362 --- /dev/null +++ b/agent/services/personas/audit_generalist.py @@ -0,0 +1,11 @@ +from .base import BasePersona +from agent.services.prompts.audit_prompt import AUDIT_PROMPT + + +class AuditGeneralist(BasePersona): + def __init__(self, api_key: str, model: str): + super().__init__(name="AuditGeneralist", api_key=api_key, model=model) + + def get_system_prompt(self) -> str: + # Reuse the consolidated audit prompt so this persona can provide a holistic pass. + return AUDIT_PROMPT diff --git a/agent/services/personas/base.py b/agent/services/personas/base.py index 710c118..5056f18 100644 --- a/agent/services/personas/base.py +++ b/agent/services/personas/base.py @@ -3,6 +3,8 @@ import os import json import re +import time +import random from typing import Dict, Any, List class BasePersona(ABC): @@ -191,8 +193,8 @@ def hunt( 'If no issue, return {"found_vulnerability": false}.' ) - try: - response = self.client.chat.completions.create( + def _call(): + return self.client.chat.completions.create( model=self.model, messages=[ {"role": "system", "content": system_prompt}, @@ -200,8 +202,23 @@ def hunt( ], response_format={"type": "json_object"} ) - raw = json.loads(response.choices[0].message.content) - return self._normalize_response(raw) + + try: + max_attempts = 3 + for attempt in range(1, max_attempts + 1): + try: + response = _call() + raw = json.loads(response.choices[0].message.content) + return self._normalize_response(raw) + except Exception as e: + msg = str(e).lower() + if "rate limit" in msg or "429" in msg: + if attempt == max_attempts: + raise + sleep_ms = random.randint(200, 600) * attempt + time.sleep(sleep_ms / 1000.0) + continue + raise except Exception as e: print(f"[{self.name}] Error: {e}") return {"found_vulnerability": False} diff --git a/agent/services/personas/centralization_expert.py b/agent/services/personas/centralization_expert.py deleted file mode 100644 index 064b3d7..0000000 --- a/agent/services/personas/centralization_expert.py +++ /dev/null @@ -1,73 +0,0 @@ -from .base import BasePersona - -class CentralizationExpert(BasePersona): - def __init__(self, api_key: str, model: str): - super().__init__(name="CentralizationExpert", api_key=api_key, model=model) - - def get_system_prompt(self) -> str: - return """ - You are 'The Centralization Expert', a specialist in identifying centralization risks and single points of failure. - - YOUR EXPERTISE: - 1. Single admin keys (Owner can drain funds) - 2. Multisig vulnerabilities (Insufficient signers, key compromise) - 3. Upgradeable contract risks (Admin can change logic) - 4. Timelock absence (No delay for critical operations) - 5. Governance centralization (Whale control, flash loan attacks) - - KEY PATTERNS TO DETECT: - - Single owner with unlimited power - - Missing timelock on critical functions - - Insufficient multisig threshold - - Upgradeable contracts without governance - - Centralized oracles/data sources - - REAL-WORLD EXPLOITS: - - Ronin Bridge (2022): $625M - 5 of 9 validator keys compromised - - Multiple protocols with single admin key risks - - RESEARCH RESOURCES: - - Solodit: Filter by "Centralization Risk" - - Code4rena/Sherlock: Common audit findings - - Audit reports on governance mechanisms - - VULNERABLE PATTERNS: - // Single owner risk - address public owner; - function withdrawAll() public onlyOwner { - payable(owner).transfer(address(this).balance); // ❌ Single point of failure - } - - // No timelock - function changeCriticalParam(uint newValue) public onlyOwner { - criticalParam = newValue; // ❌ Immediate change, no delay - } - - // Insufficient multisig - require(signatures.length >= 2, "Need 2 signatures"); // ❌ Too few signers - - SECURE PATTERNS: - // Multisig with timelock - function withdrawAll() public { - require(hasRole(ADMIN_ROLE, msg.sender), "Not admin"); - require(block.timestamp >= proposedTime + TIMELOCK, "Timelock not passed"); // βœ… - payable(treasury).transfer(address(this).balance); - } - - // Governance-based changes - function changeCriticalParam(uint newValue) public { - require(governance.hasVoted(newValue), "Not voted"); // βœ… Governance required - criticalParam = newValue; - } - - Output JSON: - { - "found_vulnerability": true, - "title": "Centralization Risk", - "severity": "Medium/High", - "kill_chain": "Step 1: Admin key compromised. Step 2: Attacker drains funds...", - "centralization_type": "single-admin|insufficient-multisig|no-timelock|governance-risk|oracle-centralization" - } - - If safe, output: {"found_vulnerability": false} - """ diff --git a/agent/services/personas/logic_expert.py b/agent/services/personas/logic_expert.py index bfe7137..5102ef6 100644 --- a/agent/services/personas/logic_expert.py +++ b/agent/services/personas/logic_expert.py @@ -6,84 +6,37 @@ def __init__(self, api_key: str, model: str): def get_system_prompt(self) -> str: return """ - You are 'The Logic Expert', a specialist in business logic flaws and edge case vulnerabilities. - - YOUR EXPERTISE: - 1. Business logic errors (Incorrect protocol behavior) - 2. Edge case handling (Boundary conditions) - 3. State machine violations - 4. Incorrect assumptions about external contracts - 5. Race conditions in complex protocols - - KEY PATTERNS TO DETECT: - - Incorrect state transitions - - Missing edge case validation - - Logic errors in DeFi protocols - - Incorrect calculations in complex systems - - Assumptions about external contract behavior - - Missing validation for boundary conditions - - REAL-WORLD EXPLOITS: - - Multiple DeFi protocols with logic bugs - - Complex protocol interactions causing unexpected behavior - - RESEARCH RESOURCES: - - Solodit: 40,000+ findings (many are logic errors) - - Web3Bugs: Real logic errors from Code4rena, Sherlock - - DeFiHackLabs: Business logic flaws in DeFi protocols - - GitHub: https://github.com/ZhangZhuoSJTU/Web3Bugs - - GitHub: https://github.com/SunWeb3Sec/DeFiHackLabs - - VULNERABLE PATTERNS: - // Missing edge case - function calculateReward(uint staked, uint time) public pure returns (uint) { - return staked * time / 365; // ❌ Division by zero if time = 0 - } - - // Incorrect state logic - function withdraw() public { - require(balance > 0); - // ❌ Missing check: is withdrawal enabled? - // ❌ Missing check: is user locked? - transfer(msg.sender, balance); - } - - // Assumption about external contract - function swap(address token) public { - uint balance = IERC20(token).balanceOf(address(this)); - // ❌ Assumes token returns true on transfer - // ❌ Assumes token has no fees - } - - SECURE PATTERNS: - function calculateReward(uint staked, uint time) public pure returns (uint) { - require(time > 0, "Time must be positive"); // βœ… Edge case - return staked * time / 365; - } - - function withdraw() public { - require(balance > 0, "No balance"); - require(!isLocked[msg.sender], "User locked"); // βœ… State check - require(withdrawalsEnabled, "Withdrawals disabled"); // βœ… Feature flag - transfer(msg.sender, balance); - } - - function swap(address token) public { - uint balanceBefore = IERC20(token).balanceOf(address(this)); - IERC20(token).transferFrom(msg.sender, address(this), amount); - uint balanceAfter = IERC20(token).balanceOf(address(this)); - uint received = balanceAfter - balanceBefore; // βœ… Handle fee-on-transfer - require(received >= minAmount, "Slippage"); - } - - Output JSON: + You are 'The Logic Expert', a specialist in business logic flaws, state machines, and edge cases. + + ANALYSIS METHODOLOGY: + 1) Identify critical functions (public/external/payable) and their state transitions. + 2) Check invariants: enable/disable flags, locking, role checks, balance/allowance consistency, supply math. + 3) Enumerate edge cases: zero amounts, max values, repeated calls, partial fills, paused/disabled states. + 4) Verify external assumptions: token return values, fee-on-transfer, rebasing tokens, oracle freshness, slippage bounds. + 5) Flag only with concrete failure: cite the lines and functions where checks are missing or invariants break; show the exploit path. + + COMMON FALSE POSITIVES TO AVOID: + - Properly guarded feature flags or role checks. + - Safe math in Solidity >=0.8 unless inside unchecked. + - Reentrancy claims when CEI and/or nonReentrant are present. + - View/pure functions or functions that do not mutate critical state. + + OUTPUT JSON (single object): { - "found_vulnerability": true, - "title": "Logic Error / Edge Case", - "severity": "High/Medium/Low", - "kill_chain": "Step 1: Attacker exploits edge case. Step 2: Breaks protocol logic...", - "logic_type": "business-logic|edge-case|state-machine|external-assumption|race-condition" + "found_vulnerability": boolean, + "title": "Logic Error / Edge Case", + "severity": "Critical|High|Medium|Low|Informational", + "line_number": integer, + "affected_functions": ["funcName"], + "affected_lines": [int], + "confidence_score": 0-100, + "false_positive_risk": 0-100, + "description": "Concrete flaw and remediation; include function/context.", + "attack_logic": "Step-by-step exploit narrative", + "verification_proof": "Reasoning or PoC", + "proof_of_concept": "Optional code", + "logic_type": "business-logic|edge-case|state-machine|external-assumption|race-condition" } - - If safe, output: {"found_vulnerability": false} + + If safe, return {"found_vulnerability": false}. """ diff --git a/agent/services/personas/thief.py b/agent/services/personas/thief.py deleted file mode 100644 index b5368c3..0000000 --- a/agent/services/personas/thief.py +++ /dev/null @@ -1,24 +0,0 @@ -from .base import BasePersona - -class Thief(BasePersona): - def __init__(self, api_key: str, model: str): - super().__init__(name="Thief", api_key=api_key, model=model) - - def get_system_prompt(self) -> str: - return """ - You are 'The Thief', a specialist in Access Control and Asset Draining. - - YOUR TARGETS: - 1. Public 'init' or 'initialize' functions (Privilege Escalation). - 2. Functions that withdraw ETH/Tokens without 'onlyOwner'. - - Output JSON: - { - "found_vulnerability": true, - "title": "Vulnerability Name", - "severity": "Critical", - "kill_chain": "Step 1: I call init(). Step 2: I become owner..." - } - - If safe, output: {"found_vulnerability": false} - """ diff --git a/agent/services/prompts/audit_prompt.py b/agent/services/prompts/audit_prompt.py index 0aa42e3..8a67d7e 100644 --- a/agent/services/prompts/audit_prompt.py +++ b/agent/services/prompts/audit_prompt.py @@ -6,7 +6,7 @@ 2) For each issue, return a single JSON object with the required fields. 3) Include the most relevant line number (best guess if unsure) and function/context. 4) Keep severity to: Critical, High, Medium, Low, Informational. -5) If nothing is found, return {"findings": []}. +5) If nothing is found, return {{\"findings\": []}}. ## Vulnerability Categories To Consider - Reentrancy (check-effects-interactions) diff --git a/agent/services/reference_docs.py b/agent/services/reference_docs.py new file mode 100644 index 0000000..771b1f1 --- /dev/null +++ b/agent/services/reference_docs.py @@ -0,0 +1,19 @@ +""" +Lightweight built-in reference docs passed to personas to reduce false positives and +provide shared best practices without requiring external input. +Keep concise to avoid prompt bloat. +""" + +DEFAULT_DOCS = """ +Persona Reference Notes: +- Reentrancy: CEI (checks-effects-interactions) with nonReentrant is usually safe. Pull payments safer than push. +- Arithmetic: Solidity >=0.8 reverts on overflow/underflow unless inside unchecked. SafeMath adds guards for <0.8. +- Access Control: onlyOwner/roles/timelocks/multisigs mitigate centralization; do not flag when properly used. +- Proxies/Upgrades: transparent/beacon proxies should restrict admin/implementation changes; storage layouts must align. +- Tokens: ERC20 transfer/transferFrom may return false; some tokens are fee-on-transfer or rebasingβ€”handle received amount. +- External Calls: low-level call must check success and bubble errors; avoid delegatecall to untrusted addresses. +- Oracles: check freshness (updatedAt) and stale/zero values; consider price deviation bounds. +- Timestamp/Block: avoid using block.timestamp/block.number for critical randomness; allow small drift tolerances. +- Validation: guard zero address, bounds (min/max), array length alignment, and zero amounts where unsafe. +- Gas: avoid SLOAD/SSTORE in tight loops, pack variables, cache storage reads, and avoid redundant checks in pure/view paths. +""" diff --git a/agent/services/swarm.py b/agent/services/swarm.py index de06824..1545108 100644 --- a/agent/services/swarm.py +++ b/agent/services/swarm.py @@ -7,7 +7,6 @@ from typing import List, Dict, Any, Set, Type, Optional from .personas.access_control_expert import AccessControlExpert from .personas.arithmetic_expert import ArithmeticExpert -from .personas.centralization_expert import CentralizationExpert from .personas.defi_analyst import DeFiAnalyst from .personas.dos_expert import DoSExpert from .personas.economic_expert import EconomicExpert @@ -22,11 +21,11 @@ from .personas.reentrancy_expert import ReentrancyExpert from .personas.signature_expert import SignatureExpert from .personas.storage_proxy_expert import StorageProxyExpert -from .personas.thief import Thief from .personas.timestamp_expert import TimestampExpert from .personas.token_expert import TokenExpert from .personas.routing_analyst import RoutingAnalyst from .personas.critic import Critic +from .personas.audit_generalist import AuditGeneralist logger = logging.getLogger(__name__) @@ -34,23 +33,29 @@ class Swarm: def __init__( self, api_key: str = None, - model: str = "gpt-4o", + model: str = "gpt-4.1-mini", cache_enabled: bool = False, persona_models: Optional[Dict[str, str]] = None, - routing_enabled: bool = False, + routing_enabled: bool = True, ): # The Council of Agents # Add new personas here as you build them - self.persona_models = persona_models or {} + # Normalize model choice to avoid weaker defaults like gpt-4o-mini. + normalized_model = model or "gpt-4.1-mini" + if normalized_model.lower() in {"gpt-4o-mini", "gpt-4o-mini-2024-07-18"}: + normalized_model = "gpt-4.1-mini" + + self.persona_models = { + k: ("gpt-4.1-mini" if v and v.lower().startswith("gpt-4o-mini") else v) + for k, v in (persona_models or {}).items() + } def _select_model(cls): - return self.persona_models.get(cls.__name__, model) + return self.persona_models.get(cls.__name__, normalized_model) self.agents = [ - Thief(api_key=api_key, model=_select_model(Thief)), AccessControlExpert(api_key=api_key, model=_select_model(AccessControlExpert)), ArithmeticExpert(api_key=api_key, model=_select_model(ArithmeticExpert)), - CentralizationExpert(api_key=api_key, model=_select_model(CentralizationExpert)), DeFiAnalyst(api_key=api_key, model=_select_model(DeFiAnalyst)), DoSExpert(api_key=api_key, model=_select_model(DoSExpert)), EconomicExpert(api_key=api_key, model=_select_model(EconomicExpert)), @@ -68,13 +73,12 @@ def _select_model(cls): TimestampExpert(api_key=api_key, model=_select_model(TimestampExpert)), TokenExpert(api_key=api_key, model=_select_model(TokenExpert)), Critic(api_key=api_key, model=_select_model(Critic)), + AuditGeneralist(api_key=api_key, model=_select_model(AuditGeneralist)), ] self._agent_by_type = {type(agent): agent for agent in self.agents} self._persona_name_to_type: Dict[str, Type] = { - "Thief": Thief, "AccessControlExpert": AccessControlExpert, "ArithmeticExpert": ArithmeticExpert, - "CentralizationExpert": CentralizationExpert, "DeFiAnalyst": DeFiAnalyst, "DoSExpert": DoSExpert, "EconomicExpert": EconomicExpert, @@ -92,6 +96,7 @@ def _select_model(cls): "TimestampExpert": TimestampExpert, "TokenExpert": TokenExpert, "Critic": Critic, + "AuditGeneralist": AuditGeneralist, } self.routing_analyst = RoutingAnalyst(api_key=api_key, model=model) # In-memory cache keyed by content hash to skip re-analysis of unchanged files. @@ -118,13 +123,13 @@ def has_any(substrings: List[str]) -> bool: return any(s in code_lower for s in substrings) always_on: Set[Type] = { - Thief, AccessControlExpert, ReentrancyExpert, LogicExpert, DeFiAnalyst, GasOptimizationExpert, Critic, + AuditGeneralist, } heuristic_hits: Set[str] = set() @@ -205,9 +210,9 @@ def has_any(substrings: List[str]) -> bool: ) return selected - def _code_snippet(self, source_code: str, line_number: int, context: int = 12) -> str: + def _code_snippet(self, source_code: str, line_number: int, window: int = 10) -> str: """ - Returns a small, line-numbered snippet around the reported line for triage. + Returns a small, line-numbered snippet (max `window` lines) centered on the reported line. """ if not line_number or line_number < 1: return "" @@ -215,8 +220,9 @@ def _code_snippet(self, source_code: str, line_number: int, context: int = 12) - idx = line_number - 1 if idx >= len(lines): return "" - start = max(0, idx - context) - end = min(len(lines), idx + context + 1) + half = max(1, window // 2) + start = max(0, idx - half) + end = min(len(lines), start + window) snippet_lines = [f"{i + 1}: {lines[i][:400]}" for i in range(start, end)] return "\n".join(snippet_lines) @@ -261,7 +267,7 @@ def _run_agent(agent): logger.exception("Agent %s failed during hunt on %s", agent.name, filename) return agent, {} - max_workers = len(selected_agents) or 1 + max_workers = min(len(selected_agents) or 1, 8) with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: for agent, analysis in executor.map(_run_agent, selected_agents): if persona_outputs is not None: diff --git a/benchmarks/last_benchmark.json b/benchmarks/last_benchmark.json new file mode 100644 index 0000000..174b3fc --- /dev/null +++ b/benchmarks/last_benchmark.json @@ -0,0 +1,1404 @@ +{ + "timestamp": "2025-11-23T14:09:14.401658Z", + "raw_persona_outputs": [ + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Misuse of Public Functions for Critical State Changes and Missing Access Controls", + "description": "The functions _addCredIdPerAddress and _removeCredIdPerAddress are declared as 'public' but are internal functions intended to only be called within the contract's logic. Being 'public', they are externally callable and risk abuse. An attacker could invoke these functions to manipulate internal data structures, leading to potential privilege escalation or data corruption, especially since no access control modifiers protect these functions. This exposure could allow an attacker to forge share ownership or delete cred associations, affecting contract integrity.", + "attack_logic": "An attacker calls _addCredIdPerAddress or _removeCredIdPerAddress directly via transaction, altering internal share tracking data to manipulate access or ownership states. For example, they could remove their cred IDs or inject invalid cred IDs, leading to misleading share balances or denial of proper access controls.", + "verification_proof": "Lines 245 and 278 declare functions as 'public' that are meant to be internal helpers. Solidity exposes these functions publicly if marked 'public'. No access control modifiers such as 'internal' or 'private' are used. This design flaw permits external calls to internal logic functions, violating access restrictions.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "_addCredIdPerAddress", + "_removeCredIdPerAddress" + ], + "affected_lines": [ + 245, + 278 + ], + "severity": "Critical", + "line_number": 245, + "confidence_score": 90, + "false_positive_risk": 0 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": true, + "title": "Gas Optimization Opportunity in Internal Functions and Storage Usage", + "description": "The functions `_addCredIdPerAddress` and `_removeCredIdPerAddress` repeatedly access and modify storage mappings and arrays without minimal batching or packing. For example, using the `push()` and `pop()` methods with multiple mappings (e.g., `_credIdsPerAddress`, `_credIdExistsPerAddress`, `_credIdsPerAddressCredIdIndex`) and incrementing/decrementing counters separately incurs extra gas costs. By packing related variables or reducing storage reads/writes, significant gas savings are possible. Also, marking `_addCredIdPerAddress` and `_removeCredIdPerAddress` as `internal` (already is) and ensuring no redundant operations further optimizes gas.", + "attack_logic": "This is an optimization, not a vulnerability. Optimized storage handling reduces gas costs for functions managing curator cred IDs.", + "verification_proof": "The functions perform multiple storage reads/writes in loops for managing curator cred IDs. Refactoring to optimize storage access patterns can reduce gas.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "_updateCuratorShareBalance", + "_addCredIdPerAddress", + "_removeCredIdPerAddress" + ], + "affected_lines": [ + 536, + 579, + 612 + ], + "severity": "Informational", + "line_number": 524, + "confidence_score": 85, + "false_positive_risk": 15 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Potential Reentrancy via _handleTrade in _executeBatchTrade", + "description": "The function _executeBatchTrade updates balances and then transfers ETH to external addresses (protocolFeeDestination and via depost). Even though the function is protected with nonReentrant modifier, a reentrancy risk remains if external code reenters before the state updates due to insufficient order or gaps in guard logic. Notably, the external calls to safeTransferETH are made after state updates, but the presence of multiple external calls in a loop and the recursion potential via fallback functions warrants caution. The pattern relies on nonReentrant, which appears to be correctly applied at the function level, but complex batch processes involving multiple external calls could still pose risks if any reentrancy guard is bypassed or if other functions invoked externally are not protected.", + "attack_logic": "An attacker could craft a malicious contract with fallback functions that reenter the _executeBatchTrade as soon as ETH transfer occurs, possibly exploiting state inconsistency if the reentrancy guard is bypassed or if any external call responds unexpectedly. Given that the function uses nonReentrant, the risk is mitigated at the code level, but complex interaction and external contract behaviors could potentially cause issues if the guard fails under certain conditions.", + "verification_proof": "The nonReentrant modifier is applied at _executeBatchTrade, which should prevent reentrancy, but the risk remains if the guard is circumvented or if external calls have unexpected behaviors. The pattern is standard but requires cautious external interaction, especially with batch loops involving multiple external calls.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "_executeBatchTrade" + ], + "affected_lines": [ + 803 + ], + "severity": "Critical", + "line_number": 803, + "confidence_score": 85, + "false_positive_risk": 10 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Unprotected withdraw function allows potential abuse", + "description": "The `withdraw()` function is marked as external onlyOwner and directly transfers all ETH in the contract to `protocolFeeDestination`. While access is restricted to owner, it is a potentially critical single escape or misuse point if ownership is compromised. No reentrancy guard is used here, but since the function only transfers ETH to a controlled address, the risk is low in practice. Still, it's worth noting for mitigation and best practices.", + "attack_logic": "An attacker with owner access could maliciously withdraw funds at any time, but since only owner can call this, direct exploitation isn't possible externally. However, if ownership is compromised, this function allows draining all ETH from the contract, leading to total loss of protocol funds.", + "verification_proof": "The `withdraw()` function on line 454 is only callable by owner (`onlyOwner`), and it executes a safetransferETH to the `protocolFeeDestination`, which is a controlled address. The only concern is the consequence of ownership compromise, not a direct vulnerability.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "withdraw" + ], + "affected_lines": [ + 454 + ], + "severity": "High", + "line_number": 454, + "confidence_score": 95, + "false_positive_risk": 5 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": true, + "title": "Unbounded Merkle Root Hash Update Vulnerability", + "description": "The contract allows the owner to directly set or update the merkle root hash in credMerkleRoot[credChainId][credId] without any access control or validation beyond owner-only, with no upper bound or timing restrictions. This can enable malicious or mistaken updates to the Merkle root, potentially invalidating proofs or allowing assertion of false claims. The lack of a mechanism to restrict or log such changes can lead to trust issues or exploitation by the owner.", + "attack_logic": "An attacker (or compromised owner) updates the Merkle root to a malicious value, causing all subsequent merkleClaim validations to succeed with fraudulent data, or to invalidate legitimate claims, disrupting the credentialing process or enabling fraudulent claims under the guise of valid proofs.", + "verification_proof": "Line 729 shows direct owner-only access to set credMerkleRoot[credChainId][credId], with no validation or update restrictions. Changing this root arbitrarily influences merkleClaim validation outcomes.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "setCredMerkleRoot" + ], + "affected_lines": [ + 729 + ], + "severity": "High", + "line_number": 729, + "confidence_score": 85, + "false_positive_risk": 10 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": true, + "title": "Lack of upper bound check for mintFee in createArt", + "description": "The createArt function does not impose an upper limit on the artCreateFee or other fee-related parameters, allowing a malicious user to set an arbitrarily high fee. This fee could potentially cause integer overflow in other parts if combined with fee calculations elsewhere. Although Solidity >=0.8 has safe math by default, the primary risk is economic (charging excessively high fees). The issue is that there are no bounds or validation on createConfig_.mintFee or artCreateFee_ in setArtCreatFee and createArt, respectively, enabling fee manipulation.", + "attack_logic": "A malicious creator could set an extremely high artCreateFee or mintFee, causing them to extract large amounts of ETH during creation or subsequent vaults without proper bounds, or trigger overflow if combined improperly elsewhere. This can be exploited to drain ETH or cause economic harm.", + "verification_proof": "No explicit maximum fee checks exist in setArtCreatFee or createArt functions; only a max cap check (e.g., >10,000) is in setProtocolFee, but not in createArt's input parameter for fee.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "createArt" + ], + "affected_lines": [ + 134 + ], + "severity": "High", + "line_number": 134, + "confidence_score": 85, + "false_positive_risk": 5 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": true, + "title": "Missing ownership control over critical setter functions", + "description": "The setter functions for critical addresses and fee parameters are protected only by the onlyOwner modifier. However, the owner can be maliciously or maliciously compromised, leading to control over fee and address configurations, potentially causing misappropriation of funds or denial of proper contract operation. These functions are correctly restricted, but the attack would depend on the security of the owner key. Additionally, the function 'setArtCreatFee' is misspelled as 'setArtCreatFee' (line 370), causing potential confusion or misusage. Consider implementing multi-sig or timelock for critical parameter changes.", + "attack_logic": "An attacker with owner access can change critical parameters (e.g., protocolFee, addresses). Malicious owner can redirect fees, set invalid addresses, or disable proper fee collection. Since onlyOwner restricts access, the main concern is owner key compromise.", + "verification_proof": "Owner-only functions are correctly guarded, but their misuse or theft directly impacts protocol integrity.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "setPhiSignerAddress", + "setPhiRewardsAddress", + "setErc1155ArtAddress", + "setProtocolFeeDestination", + "setProtocolFee", + "setArtCreatFee" + ], + "affected_lines": [ + 343, + 350, + 357, + 364, + 370, + 377 + ], + "severity": "High", + "line_number": 343, + "confidence_score": 95, + "false_positive_risk": 0 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Unprotected Payable External Functions Allowing Unauthorized Calls", + "description": "Both 'signatureClaim' and 'merkleClaim' are external payable functions called by anyone and are not protected by access control modifiers such as 'onlyOwner' or similar. This design indicates they are intended for public claim submissions, but if any internal functions or state variables are manipulated or if any further logic (e.g., token transfers) are embedded in the called functions of 'IPhiFactory' that are callable by anyone, it can lead to unauthorized claims or replay attacks. The risk comes from public payable functions that process claims without verification of the caller's authorization. These functions rely on off-chain or external verification methods, but the current code itself lacks any access restrictions.", + "attack_logic": "A malicious user could repeatedly invoke 'signatureClaim' or 'merkleClaim' with crafted input data, potentially leading to overclaiming, reentrancy in downstream contracts, or claiming rewards multiple times without proper state checks if those are managed off-chain or within called contracts.", + "verification_proof": "The functions 'signatureClaim' and 'merkleClaim' are public, payable, and lack access control modifiers. Their logic depends solely on parameters passed by callers. Since there are no modifiers restricting who can call them, they are inherently vulnerable to spam, replay, or abuse if the called functions inside 'IPhiFactory' do not themselves enforce claim limits.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "signatureClaim", + "merkleClaim" + ], + "affected_lines": [ + 51, + 69 + ], + "severity": "Critical", + "line_number": 51, + "confidence_score": 90, + "false_positive_risk": 10 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "No significant vulnerabilities detected", + "description": "The Claimable contract exposes externally callable, payable functions that delegate calls to the factory. The functions rely on external signature verification and Merkle proof validation, which appear to have proper decodings and input validation. No unsafe external call patterns, reentrancy, or arithmetic issues are present. Signature and Merkle proof mechanisms are standard, and no obvious logic flaws are apparent within the scope of this code.", + "attack_logic": "None identified; the functions depend on external validated data and interfaces with no common exploits apparent.", + "verification_proof": "The functions are simple wrappers delegating to factory methods with proper decoding and input validation. No reentrancy or state manipulation in this contract.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "Informational", + "line_number": 0, + "confidence_score": 100, + "false_positive_risk": 0 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "InvalidMerkleClaimData revert allows unprotected decoding", + "description": "The `_decodeMerkleClaimData` function strictly relies on `msg.data.length` to prevent malformed inputs. If an external caller crafts a `msg.data` with length >=260 but still with invalid encoded data (e.g., partially missing fields, malformed concatenation), the `abi.decode` operation may revert unexpectedly. Additionally, since this function is `private` and only called internally by `merkleClaim()`, the attack surface is limited, but the check on data length does not guarantee correctness of all encoded inputs, e.g., a malicious user could cause a revert that may affect the calling `merkleClaim()` logic.", + "attack_logic": "An attacker crafts a `msg.data` with length at least 260 bytes but with incorrect internal encoding, causing `_decodeMerkleClaimData()` to revert. This potential revert could disrupt `merkleClaim()`, leading to denial of service or unhandled revert states in the claiming process.", + "verification_proof": "The `abi.decode` in `_decodeMerkleClaimData()` reverts if data does not match expected encoding structure, even if length check passes. Malformed or malicious data can cause revert, as per Solidity 0.8.25 behavior.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "_decodeMerkleClaimData" + ], + "affected_lines": [ + 47 + ], + "severity": "Medium", + "line_number": 47, + "confidence_score": 80, + "false_positive_risk": 10 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Unchecked msg.data Length for merkleClaim decoding", + "description": "The contract performs a length check (msg.data.length < 260) before decoding in _decodeMerkleClaimData (line 42). However, the check may be insufficient if the input data is manipulated to be exactly 260 bytes or greater but invalid, leading to an unchecked decode. An attacker might craft a malicious payload with precise length and invalid data to cause decode failure or unexpected behavior.", + "attack_logic": "An attacker crafts msg.data with length exactly 260 bytes (or more) but with malformed or malicious payload. The decode operation may revert or produce incorrect data, potentially causing the claim process to fail or misbehave. Since the length check only enforces a minimum size but not maximum or validity of data, this could lead to unexpected errors.", + "verification_proof": "The length check (msg.data.length < 260) is a minimal safeguard, but does not prevent malformed inputs of exactly 260 bytes. Additional validation or safe decoding patterns are recommended to mitigate this risk.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "_decodeMerkleClaimData" + ], + "affected_lines": [ + 42 + ], + "severity": "High", + "line_number": 42, + "confidence_score": 90, + "false_positive_risk": 10 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "No Access Control Vulnerabilities Detected", + "description": "All externally callable functions are either view or internal. \n- initializeRoyalties is internal, preventing external calls.\n- getRoyalties and royaltyInfo are view functions that do not change state.\n- supportsInterface is a standard view function.\nThere are no public or external functions that permit state modifications without proper access control. \nThe pattern aligns with best practices; no accessible functions allow privilege escalation or unprotected state changes.", + "attack_logic": "N/A", + "verification_proof": "All public functions are either readonly (view) or internal, preventing external unauthorized state modifications.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "initializeRoyalties", + "getRoyalties", + "royaltyInfo", + "_updateRoyalties", + "supportsInterface" + ], + "affected_lines": [ + 12, + 20, + 32, + 38, + 48 + ], + "severity": "Low", + "line_number": 12, + "confidence_score": 90, + "false_positive_risk": 80 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "No apparent vulnerabilities in royalty info contract", + "description": "The contract provides standard ERC2981 royalty implementation with controlled initialization. No external or payble functions, and no state-changing functions are exposed publicly that could lead to reentrancy or manipulation vulnerabilities. The internal functions for updating and retrieving royalties are secured by access controls (presumably in inheriting contracts), and interface support is standard.", + "attack_logic": "Not applicable; no external payable or state-changing public functions accessible by attackers.", + "verification_proof": "The contract implements ERC2981 interface support and provides getter functions with no external state-altering code exposed publicly, reducing attack surface.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "Low", + "line_number": 0, + "confidence_score": 85, + "false_positive_risk": 10 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "No Reentrancy or Vulnerability Detected", + "description": "The contract only exposes view functions (`getRoyalties`, `royaltyInfo`, and `supportsInterface`) which are not susceptible to reentrancy or state-changing exploits. The internal state variables are only set during initialization and updates, with no external calls or payable functions present. Checks like `revert` statements are in place to prevent invalid states. No external or public functions are vulnerable to reentrancy, and arithmetic is safe due to Solidity 0.8.25's inbuilt checks.", + "attack_logic": "Not applicable as the contract does not have mutable external functions or payable functions that could be re-entered.", + "verification_proof": "All functions are view and internal, with no external calls, external state mutations, or payable functions; thus, no attack surface for reentrancy.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "Low", + "line_number": 0, + "confidence_score": 90, + "false_positive_risk": 10 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Public Payable External Functions with State Changes (Potential Re-Entrancy & Access Risks)", + "description": "Both 'createArtFromFactory' and 'claimFromFactory' are external, payable functions that perform state changes and ETH transfers without re-entrancy guards or explicit access control beyond 'onlyPhiFactory'. These functions involve ETH transfers before updating state or minting tokens, exposing potential re-entrancy risks. Although 'ReentrancyGuard' is not used, the transfer of ETH to external addresses makes these functions vulnerable if external contracts are malicious. Additional protective measures like re-entrancy guards and explicit access control are recommended.", + "attack_logic": "An attacker-controlled contract can call 'createArtFromFactory' or 'claimFromFactory', invoke a callback via fallback or receive ETH, and re-enter to exploit state changes or siphon funds, especially since ETH is sent before finalizing the state or minting, leading to potential re-entrancy vulnerabilities.", + "verification_proof": "Lines 112 ('createArtFromFactory') and 131 ('claimFromFactory') send ETH before state updates or token minting, with no re-entrancy guard applied, making re-entrancy possible if called with malicious fallback/receive functions.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "createArtFromFactory", + "claimFromFactory" + ], + "affected_lines": [ + 112, + 131 + ], + "severity": "High", + "line_number": 112, + "confidence_score": 85, + "false_positive_risk": 10 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "No access control vulnerabilities detected", + "description": "The contract's external and public functions are either view or non-state-changing, with access controls properly enforced (e.g., 'onlyOwner' on 'setCredContract'). Payable functions are absent, and no public or external functions are unprotected. The constructor uses 'Ownable' to set the owner, and functions with potential state changes are restricted appropriately.", + "attack_logic": "Not applicable, no vulnerabilities found.", + "verification_proof": "All externally callable functions are either view or restricted via 'onlyOwner' or internal, with no unprotected state-changing functions exposed.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "Low", + "line_number": 0, + "confidence_score": 80, + "false_positive_risk": 0 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Likely False Positive", + "description": "This contract only exposes getter and setter functions for address references and pure/view price calculations. No state-changing or external calls are made without appropriate access controls, and it uses Solidity 0.8.25 with built-in overflow checks. No reentrancy guards are needed here. Therefore, no vulnerability is present.", + "attack_logic": "N/A", + "verification_proof": "No state-changing functions with external calls or unprotected user input. Price calculations are pure/view operations with no known attack vectors.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "Informational", + "line_number": 0, + "confidence_score": 0, + "false_positive_risk": 0 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Public External Functions Without Access Control", + "description": "The functions 'updatePhiRewardsContract', 'updateRoyalty', 'deposit', and 'distribute' are marked as 'external' and 'public' but lack any access modifier restricting who can call them. Since they are not marked with onlyOwner or other role-based modifiers, anyone can invoke these functions, leading to potential unauthorized updates or manipulations of contract state. This is especially critical for 'deposit' (line 39), which is payable and handles ETH deposits for distribution, and 'distribute' (line 54), which triggers reward distributions. The owner is intended to control these sensitive operations, but access control is missing.", + "attack_logic": "An attacker can call 'distribute' repeatedly, drain or manipulate reward distributions, or invoke 'updateRoyalty' and 'updatePhiRewardsContract' to redirect funds or misconfigure the contract without authorization. In particular, calling 'deposit' repeatedly could manipulate the contract\u2019s state or allow malicious ETH deposits. Since no access control exists, these functions are open to abuse.", + "verification_proof": "Lines 23, 30, 39, and 54 are declared as 'external' functions without 'onlyOwner' or similar access modifiers, exposing critical state-changing operations publicly. The functions do not check roles or ownership, allowing anyone to call them.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "updatePhiRewardsContract", + "updateRoyalty", + "deposit", + "distribute" + ], + "affected_lines": [ + 23, + 30, + 39, + 54 + ], + "severity": "Critical", + "line_number": 23, + "confidence_score": 95, + "false_positive_risk": 10 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Potential Reentrancy and ETH Handling Issue in distribute()", + "description": "The `distribute()` function performs an external call `_msgSender().safeTransferETH()` before the internal state update `balanceOf[credId] -= totalBalance`. Although `safeTransferETH()` is a protected, well-implemented function that is unlikely to reenter, the sequence of external call before state change presents a classical risk pattern. If `safeTransferETH()` delegating to an untrusted contract, reentrancy could occur, allowing malicious recipients to call `distribute()` again before the balance is zeroed, leading to multiple withdrawals.", + "attack_logic": "An attacker provides a recipient contract that executes malicious code in its fallback/receive function upon ETH receipt. On receiving ETH during `safeTransferETH()`, it reenters `distribute()` before `balanceOf[credId]` is zeroed, obtaining multiple distributions for the same balance.", + "verification_proof": "External call `_msgSender().safeTransferETH()` occurs before state update `balanceOf[credId] -= totalBalance`. Reentrancy is possible if `safeTransferETH()` delegates to a malicious fallback contract, given no reentrancy guard or Checks-Effects-Interactions pattern.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "distribute" + ], + "affected_lines": [ + 61, + 68, + 76 + ], + "severity": "High", + "line_number": 61, + "confidence_score": 85, + "false_positive_risk": 10 + } + }, + { + "persona": "AccessControlExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "DeFi Risk Analyst", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "GasOptimizationExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "LogicExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "ReentrancyExpert", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "Critic", + "raw": { + "found_vulnerability": false, + "optimization_opportunity": false, + "title": "Unknown Finding", + "description": "", + "attack_logic": "", + "verification_proof": null, + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [], + "affected_lines": [], + "severity": "High", + "line_number": 0, + "confidence_score": 60, + "false_positive_risk": 30 + } + }, + { + "persona": "AuditGeneralist", + "raw": { + "found_vulnerability": true, + "optimization_opportunity": false, + "title": "Potential Reentrancy vulnerability in handleRewardsAndGetValueSent due to internal external call", + "description": "The function handleRewardsAndGetValueSent allows external calls via 'depositRewards', specifically when 'chainSync_' is true and 'curatorRewardsDistributor.deposit' is called. Although the call is made with { value: curateTotalReward_ } and no external state updates occur before it, if 'curatorRewardsDistributor' is a malicious contract, it can reenter the PhiRewards contract during the deposit. Since there are no reentrancy guards or effects-before-interactions pattern, this could lead to reentrancy attacks that manipulate 'balanceOf' or other state variables.", + "attack_logic": "A malicious curatorRewardsDistributor contract is designed with fallback or receive functions that reenter 'handleRewardsAndGetValueSent' during 'deposit', potentially causing repeated deposits or draining of funds by manipulating balances or executing other malicious logic before state variables update as expected.", + "verification_proof": "The call to 'curatorRewardsDistributor.deposit' is an external call with value transfer, and no reentrancy guard is present around this call, opening the potential for reentrant attacks if 'curatorRewardsDistributor' is malicious.", + "gas_savings_estimate": null, + "kill_chain": null, + "affected_functions": [ + "handleRewardsAndGetValueSent" + ], + "affected_lines": [ + 74 + ], + "severity": "High", + "line_number": 74, + "confidence_score": 85, + "false_positive_risk": 10 + } + } + ], + "deduped_findings": [ + { + "title": "Misuse of Public Functions for Critical State Changes and Missing Access Controls", + "description": "None\n\nCode snippet:\n240: payable\n241: whenNotPaused\n242: {\n243: if (_recoverSigner(keccak256(signedData_), signature_) != phiSignerAddress) revert AddressNotSigned();\n244: (\n245: uint256 expiresIn,\n246: address sender,\n247: ,\n248: address bondingCurve,\n249: string memory credURL,\n\nAttack Logic: Not provided\nVerification Proof: Lines 245 and 278 declare functions as 'public' that are meant to be internal helpers. Solidity exposes these functions publicly if marked 'public'. No access control modifiers such as 'internal' or 'private' are used. This design flaw permits external calls to internal logic functions, violating access restrictions.\nDetected by: AccessControlExpert Persona", + "severity": "Critical", + "file_paths": [ + "src/Cred.sol" + ] + }, + { + "title": "Potential Reentrancy via _handleTrade in _executeBatchTrade", + "description": "None\n\nCode snippet:\n798: function _executeBatchSell(\n799: uint256[] calldata credIds_,\n800: uint256[] calldata amounts_,\n801: uint256[] memory prices,\n802: uint256[] memory protocolFees,\n803: uint256[] memory creatorFees\n804: )\n805: internal\n806: {\n807: _executeBatchTrade(credIds_, amounts_, _msgSender(), prices, protocolFees, creatorFees, false);\n\nAttack Logic: Not provided\nVerification Proof: The nonReentrant modifier is applied at _executeBatchTrade, which should prevent reentrancy, but the risk remains if the guard is circumvented or if external calls have unexpected behaviors. The pattern is standard but requires cautious external interaction, especially with batch loops involving multiple external calls.\nDetected by: LogicExpert Persona", + "severity": "Critical", + "file_paths": [ + "src/Cred.sol" + ] + }, + { + "title": "Unprotected Payable External Functions Allowing Unauthorized Calls", + "description": "None\n\nCode snippet:\n46: /// @notice Processes a merkle claim.\n47: function merkleClaim() external payable {\n48: (\n49: address minter,\n50: bytes32[] memory proof,\n51: address ref,\n52: uint256 tokenId,\n53: uint256 quantity,\n54: bytes32 leafPart,\n55: string memory imageURI\n\nAttack Logic: Not provided\nVerification Proof: The functions 'signatureClaim' and 'merkleClaim' are public, payable, and lack access control modifiers. Their logic depends solely on parameters passed by callers. Since there are no modifiers restricting who can call them, they are inherently vulnerable to spam, replay, or abuse if the called functions inside 'IPhiFactory' do not themselves enforce claim limits.\nDetected by: AccessControlExpert Persona", + "severity": "Critical", + "file_paths": [ + "src/abstract/Claimable.sol" + ] + }, + { + "title": "Public External Functions Without Access Control", + "description": "None\n\nCode snippet:\n18: /*//////////////////////////////////////////////////////////////\n19: USING\n20: //////////////////////////////////////////////////////////////*/\n21: using SafeTransferLib for address;\n22: \n23: /*//////////////////////////////////////////////////////////////\n24: STORAGE\n25: //////////////////////////////////////////////////////////////*/\n26: IPhiRewards public phiRewardsContract;\n27: ICred public credContract;\n\nAttack Logic: Not provided\nVerification Proof: Lines 23, 30, 39, and 54 are declared as 'external' functions without 'onlyOwner' or similar access modifiers, exposing critical state-changing operations publicly. The functions do not check roles or ownership, allowing anyone to call them.\nDetected by: AccessControlExpert Persona", + "severity": "Critical", + "file_paths": [ + "src/reward/CuratorRewardsDistributor.sol" + ] + }, + { + "title": "Unprotected withdraw function allows potential abuse", + "description": "None\n\nCode snippet:\n449: return artMinted[artId_][address_];\n450: }\n451: \n452: /// @notice Returns the address of a art contract for a given art ID.\n453: /// @param artId_ The ID of the art.\n454: /// @return The address of the art contract.\n455: function getArtAddress(uint256 artId_) external view returns (address) {\n456: return arts[artId_].artAddress;\n457: }\n458: \n\nAttack Logic: Not provided\nVerification Proof: The `withdraw()` function on line 454 is only callable by owner (`onlyOwner`), and it executes a safetransferETH to the `protocolFeeDestination`, which is a controlled address. The only concern is the consequence of ownership compromise, not a direct vulnerability.\nDetected by: AccessControlExpert Persona", + "severity": "High", + "file_paths": [ + "src/PhiFactory.sol" + ] + }, + { + "title": "Unbounded Merkle Root Hash Update Vulnerability", + "description": "None\n\nCode snippet:\n724: uint256 artId_,\n725: address minter_,\n726: address ref_,\n727: address verifier_,\n728: uint256 quantity_,\n729: bytes32 data_,\n730: string memory imageURI_,\n731: uint256 etherValue_\n732: )\n733: private\n\nAttack Logic: Not provided\nVerification Proof: Line 729 shows direct owner-only access to set credMerkleRoot[credChainId][credId], with no validation or update restrictions. Changing this root arbitrarily influences merkleClaim validation outcomes.\nDetected by: DeFi Risk Analyst Persona", + "severity": "High", + "file_paths": [ + "src/PhiFactory.sol" + ] + }, + { + "title": "Lack of upper bound check for mintFee in createArt", + "description": "None\n\nCode snippet:\n129: address protocolFeeDestination_,\n130: address erc1155ArtAddress_,\n131: address phiRewardsAddress_,\n132: address ownerAddress_,\n133: uint256 protocolFee_,\n134: uint256 artCreateFee_\n135: )\n136: external\n137: initializer\n138: {\n\nAttack Logic: Not provided\nVerification Proof: No explicit maximum fee checks exist in setArtCreatFee or createArt functions; only a max cap check (e.g., >10,000) is in setProtocolFee, but not in createArt's input parameter for fee.\nDetected by: LogicExpert Persona", + "severity": "High", + "file_paths": [ + "src/PhiFactory.sol" + ] + }, + { + "title": "Missing ownership control over critical setter functions", + "description": "None\n\nCode snippet:\n338: \n339: if (expiresIn_ <= block.timestamp) revert SignatureExpired();\n340: if (_recoverSigner(keccak256(encodeData_), signature_) != phiSignerAddress) revert AddressNotSigned();\n341: \n342: _validateAndUpdateClaimState(artId_, minter_, mintArgs_.quantity);\n343: _processClaim(artId_, minter_, ref_, verifier_, mintArgs_.quantity, data_, mintArgs_.imageURI, msg.value);\n344: \n345: emit ArtClaimedData(artId_, \"SIGNATURE\", minter_, ref_, verifier_, arts[artId_].artAddress, mintArgs_.quantity);\n346: }\n347: \n\nAttack Logic: Not provided\nVerification Proof: Owner-only functions are correctly guarded, but their misuse or theft directly impacts protocol integrity.\nDetected by: AuditGeneralist Persona", + "severity": "High", + "file_paths": [ + "src/PhiFactory.sol" + ] + }, + { + "title": "Unchecked msg.data Length for merkleClaim decoding", + "description": "None\n\nCode snippet:\n37: uint256 artId = getFactoryArtId(tokenId_);\n38: bytes memory claimData_ = abi.encode(expiresIn_, minter_, ref_, verifier_, artId, block.chainid, data_);\n39: bytes memory signature = abi.encodePacked(r_, vs_);\n40: \n41: IPhiFactory phiFactoryContract = getPhiFactoryContract();\n42: IPhiFactory.MintArgs memory mintArgs_ = IPhiFactory.MintArgs(tokenId_, quantity_, imageURI_);\n43: phiFactoryContract.signatureClaim{ value: msg.value }(signature, claimData_, mintArgs_);\n44: }\n45: \n46: /// @notice Processes a merkle claim.\n\nAttack Logic: Not provided\nVerification Proof: The length check (msg.data.length < 260) is a minimal safeguard, but does not prevent malformed inputs of exactly 260 bytes. Additional validation or safe decoding patterns are recommended to mitigate this risk.\nDetected by: AuditGeneralist Persona", + "severity": "High", + "file_paths": [ + "src/abstract/Claimable.sol" + ] + }, + { + "title": "Public Payable External Functions with State Changes (Potential Re-Entrancy & Access Risks)", + "description": "None\n\nCode snippet:\n107: __Pausable_init();\n108: __ReentrancyGuard_init();\n109: initializeRoyalties(protocolFeeDestination_);\n110: \n111: tokenIdCounter = 1;\n112: \n113: credChainId = credChainId_;\n114: credId = credId_;\n115: \n116: name = string(\n\nAttack Logic: Not provided\nVerification Proof: Lines 112 ('createArtFromFactory') and 131 ('claimFromFactory') send ETH before state updates or token minting, with no re-entrancy guard applied, making re-entrancy possible if called with malicious fallback/receive functions.\nDetected by: AccessControlExpert Persona", + "severity": "High", + "file_paths": [ + "src/art/PhiNFT1155.sol" + ] + }, + { + "title": "Potential Reentrancy and ETH Handling Issue in distribute()", + "description": "None\n\nCode snippet:\n56: \n57: function updateRoyalty(uint256 newRoyalty_) external onlyOwner {\n58: if (newRoyalty_ > MAX_ROYALTY_RANGE) {\n59: revert InvalidRoyalty(newRoyalty_);\n60: }\n61: withdrawRoyalty = newRoyalty_;\n62: emit RoyaltyUpdated(newRoyalty_);\n63: }\n64: \n65: /*//////////////////////////////////////////////////////////////\n\nAttack Logic: Not provided\nVerification Proof: External call `_msgSender().safeTransferETH()` occurs before state update `balanceOf[credId] -= totalBalance`. Reentrancy is possible if `safeTransferETH()` delegates to a malicious fallback contract, given no reentrancy guard or Checks-Effects-Interactions pattern.\nDetected by: AuditGeneralist Persona", + "severity": "High", + "file_paths": [ + "src/reward/CuratorRewardsDistributor.sol" + ] + }, + { + "title": "Potential Reentrancy vulnerability in handleRewardsAndGetValueSent due to internal external call", + "description": "None\n\nCode snippet:\n69: curatorRewardsDistributor = ICuratorRewardsDistributor(curatorRewardsDistributor_);\n70: emit CuratorRewardsDistributorUpdated(curatorRewardsDistributor_);\n71: }\n72: \n73: /*//////////////////////////////////////////////////////////////\n74: EXTERNAL UPDATE\n75: //////////////////////////////////////////////////////////////*/\n76: /// @notice deposit protocol rewards\n77: /// @param credId_ Cred ID\n78: function depositRewards(\n\nAttack Logic: Not provided\nVerification Proof: The call to 'curatorRewardsDistributor.deposit' is an external call with value transfer, and no reentrancy guard is present around this call, opening the potential for reentrant attacks if 'curatorRewardsDistributor' is malicious.\nDetected by: AuditGeneralist Persona", + "severity": "High", + "file_paths": [ + "src/reward/PhiRewards.sol" + ] + }, + { + "title": "InvalidMerkleClaimData revert allows unprotected decoding", + "description": "None\n\nCode snippet:\n42: IPhiFactory.MintArgs memory mintArgs_ = IPhiFactory.MintArgs(tokenId_, quantity_, imageURI_);\n43: phiFactoryContract.signatureClaim{ value: msg.value }(signature, claimData_, mintArgs_);\n44: }\n45: \n46: /// @notice Processes a merkle claim.\n47: function merkleClaim() external payable {\n48: (\n49: address minter,\n50: bytes32[] memory proof,\n51: address ref,\n\nAttack Logic: Not provided\nVerification Proof: The `abi.decode` in `_decodeMerkleClaimData()` reverts if data does not match expected encoding structure, even if length check passes. Malformed or malicious data can cause revert, as per Solidity 0.8.25 behavior.\nDetected by: LogicExpert Persona", + "severity": "Medium", + "file_paths": [ + "src/abstract/Claimable.sol" + ] + } + ] +} \ No newline at end of file diff --git a/benchmarks/run_005.json b/benchmarks/run_005.json new file mode 100644 index 0000000..23ebcb8 --- /dev/null +++ b/benchmarks/run_005.json @@ -0,0 +1,13 @@ +{ + "repo": "https://github.com/andreitoma8/learn-solidity-hacks.git", + "duration_seconds": 423.45, + "counts": { + "Critical": 20, + "High": 0, + "Medium": 0, + "Low": 0, + "Informational": 0 + }, + "total_findings": 20, + "timestamp": "2025-11-23T09:31:48Z" +} \ No newline at end of file diff --git a/benchmarks/run_006.json b/benchmarks/run_006.json new file mode 100644 index 0000000..561508f --- /dev/null +++ b/benchmarks/run_006.json @@ -0,0 +1,13 @@ +{ + "repo": "https://github.com/andreitoma8/learn-solidity-hacks.git", + "duration_seconds": 115.38, + "counts": { + "Critical": 19, + "High": 1, + "Medium": 0, + "Low": 0, + "Informational": 0 + }, + "total_findings": 20, + "timestamp": "2025-11-23T09:47:25Z" +} \ No newline at end of file diff --git a/benchmarks/run_007.json b/benchmarks/run_007.json new file mode 100644 index 0000000..39556d4 --- /dev/null +++ b/benchmarks/run_007.json @@ -0,0 +1,13 @@ +{ + "repo": "https://github.com/andreitoma8/learn-solidity-hacks.git", + "duration_seconds": 244.25, + "counts": { + "Critical": 0, + "High": 0, + "Medium": 0, + "Low": 0, + "Informational": 0 + }, + "total_findings": 0, + "timestamp": "2025-11-23T12:37:43Z" +} \ No newline at end of file diff --git a/benchmarks/run_008.json b/benchmarks/run_008.json new file mode 100644 index 0000000..85da453 --- /dev/null +++ b/benchmarks/run_008.json @@ -0,0 +1,13 @@ +{ + "repo": "https://github.com/andreitoma8/learn-solidity-hacks.git", + "duration_seconds": 223.74, + "counts": { + "Critical": 1, + "High": 1, + "Medium": 0, + "Low": 0, + "Informational": 0 + }, + "total_findings": 2, + "timestamp": "2025-11-23T12:44:23Z" +} \ No newline at end of file