Skip to content

REFACTORING_PLAN.md

Latest commit

 

History

History
316 lines (239 loc) · 8.93 KB

REFACTORING_PLAN.md

File metadata and controls

316 lines (239 loc) · 8.93 KB

SharpVM Refactoring Plan

Generated by BaseScanner analysis on 2026-01-31

Executive Summary

Metric Value
Health Score 0/100
Files 76 (77 in compilation, 1052 unused)
Lines of Code 45,148
Classes 174
Methods 2,016
Technical Debt 239.3 hours (~30 days)
Security Vulnerabilities 79 (59 high, 4 medium, 16 low)

Phase 1: Critical Code Quality Issues

1.1 God Classes (Priority: HIGH)

These classes have extremely high LCOM (Lack of Cohesion) scores and should be split:

Class LCOM Methods Fields Recommended Split
ApiEmulator 0.86 453 69 Split by API category (kernel32, ntdll, msvcrt, user32)
WinApiEmulator 0.89 115 16 Split by subsystem (memory, file, process, registry)
RealWindowsBridge 0.96 92 12 Split by window message type
X64InstructionExecutor 0.67 85 6 Split by instruction category
VmEngine 1.00 81 2 Extract opcode handlers

Action Items:

  1. src/Blackbone/Execution/ApiEmulator.cs (741 lines in EmulateCall alone)

    • Extract Kernel32Emulator.cs
    • Extract NtdllEmulator.cs
    • Extract MsvcrtEmulator.cs
    • Extract User32Emulator.cs
    • Create IApiEmulator interface

  2. src/Runtime/WinApiEmulator.cs (291 lines in DispatchApiByName)

    • Extract MemoryApiHandler.cs
    • Extract FileApiHandler.cs
    • Extract ProcessApiHandler.cs

1.2 Long Methods (Priority: HIGH)

Methods exceeding 200 lines that need immediate extraction:

Method Lines CC File
RealModeExecutor.ExecuteOpcode 1,339 297 src/CPU/RealModeExecutor.cs:189
ApiEmulator.EmulateCall 741 582 src/Blackbone/Execution/ApiEmulator.cs:145
ApiResolver.InitializeEmulatedApis 318 1 src/Blackbone/ManualMap/ApiResolver.cs:24
WinApiEmulator.DispatchApiByName 291 101 src/Runtime/WinApiEmulator.cs:1641
X64InstructionExecutor.Execute 234 165 src/Blackbone/Execution/X64InstructionExecutor.cs:71

Refactoring Strategy:

RealModeExecutor.ExecuteOpcode -> OpcodeDispatcher + individual opcode handlers
ApiEmulator.EmulateCall -> ApiDispatcher + DLL-specific emulators
ApiResolver.InitializeEmulatedApis -> ApiRegistry with fluent API

1.3 High Cyclomatic Complexity (Priority: MEDIUM)

Methods with CC > 50 requiring simplification:

Method CC Recommendation
ApiEmulator.EmulateCall 582 Switch to dictionary dispatch pattern
RealModeExecutor.ExecuteOpcode 297 Use opcode lookup table
X64InstructionExecutor.Execute 165 Strategy pattern per instruction type
X86ToIr.TranslateInstruction 135 Instruction visitor pattern
IrToVm.EmitInstruction 102 Emit strategy per IR opcode

Phase 2: Security Vulnerabilities

2.1 Path Traversal (59 instances - HIGH)

Affected Files:

  • Program.cs (lines 30, 87, 94, 178, 424)
  • src/Blackbone/Execution/DllLoader.cs (lines 488, 497)
  • src/Blackbone/Execution/VmExecutor.cs (line 1039)

Fix Pattern:

// BEFORE (vulnerable)
if (File.Exists(userPath))

// AFTER (secure)
string canonicalPath = Path.GetFullPath(userPath);
string baseDir = Path.GetFullPath(AppDomain.CurrentDomain.BaseDirectory);
if (!canonicalPath.StartsWith(baseDir, StringComparison.OrdinalIgnoreCase))
    throw new SecurityException("Path traversal attempt detected");
if (File.Exists(canonicalPath))

2.2 Log Injection (1 instance - MEDIUM)

Location: src/Blackbone/Execution/ApiEmulator.cs:913

Fix:

// Sanitize before logging
string sanitized = input.Replace("\r", "").Replace("\n", " ");
_logger.Log(sanitized);

2.3 Missing Path Canonicalization (16 instances - LOW)

Affected Methods:

  • UniversalAnalyzer.cs:56 - parameter filePath
  • VmBytecodeDecoder.cs:88 - parameter path
  • ApiTracer.cs:271, 300 - parameter filePath
  • DllLoader.cs:68 - parameter path

Phase 3: Performance Issues

3.1 LINQ in Loops (24 instances)

Locations:

  • Program.cs:460 - OrderByDescending
  • UniversalAnalyzer.cs:162, 183 - OrderByDescending
  • ApiTracer.cs:253, 314, 317 - OrderBy

Fix Pattern:

// BEFORE
foreach (var item in collection)
{
    var sorted = items.OrderByDescending(x => x.Value);  // Recomputed each iteration
}

// AFTER
var sorted = items.OrderByDescending(x => x.Value).ToList();  // Computed once
foreach (var item in collection)
{
    // Use sorted
}

3.2 String Concatenation in Loops (10 instances)

Locations:

  • src/Blackbone/Execution/ApiEmulator.cs:7003, 7034
  • src/Blackbone/PE/PeParser.cs:160-162
  • src/VmContext.cs:235, 245
  • src/VmEngine.cs:246, 300, 317

Fix Pattern:

// BEFORE
string result = "";
foreach (var item in items)
    result += item;

// AFTER
var sb = new StringBuilder();
foreach (var item in items)
    sb.Append(item);
string result = sb.ToString();

Phase 4: Code Duplication

4.1 Exact Duplicates (70 occurrences)

Example Groups:

  • EmulateInitializeCriticalSection, EmulateConfigThreadLocale, EmulateSetNewMode - identical 5-line bodies

Action: Create generic stub handler:

private ulong EmulateNoOp(string apiName) => 0;

4.2 Structural Duplicates (185 occurrences)

Major Patterns:

  • API return value handlers (GetLastError, GetCurrentProcess, etc.)
  • X64 instruction executors (Add, Sub, Adc, Sbb, And, etc.)
  • Translator methods (TranslateMov, TranslateMovzx, TranslateMovsx, etc.)

Refactoring Strategy:

// Before: 26 nearly identical methods
void ExecuteAdd() { /* arithmetic logic */ }
void ExecuteSub() { /* arithmetic logic */ }
void ExecuteAdc() { /* arithmetic logic */ }

// After: One generic method
void ExecuteArithmetic(ArithmeticOp op, Action<ulong, ulong> compute)

Phase 5: Data Clumps

5.1 Identified Clumps

Parameters Occurrences Suggested Type
context, infoLength, infoPtr, returnLengthPtr 18 QueryInfoRequest
hWnd, lParam, msg, wParam 4 WindowMessage
data, key, type, valueName 4 RegistryValueInfo
address, newProtection, oldProtection, size 3 ProtectionRequest
address, data, length, sourceOffset 3 MemoryCopyRequest

Phase 6: Logging & Error Handling

6.1 Missing Logging (117 gaps)

Classes with 0% logging coverage (87 classes):

  • All Analyzer/Modules/*.cs classes
  • VmBytecodeDecoder.cs
  • VmAssembler.cs, VmDisassembler.cs
  • MemoryMarshaller.cs
  • ApiResolver.cs

Action: Add structured logging:

private readonly ILogger<ClassName> _logger;

public void Method()
{
    _logger.LogDebug("Entering {Method}", nameof(Method));
    // ... logic ...
    _logger.LogInformation("Completed {Method} with result {Result}", nameof(Method), result);
}

6.2 Silent Catch Blocks

Locations:

  • FileFormatDetector.cs:235 - AnalyzePE
  • ObfuscationDetector.cs:75 - DetectKnownProtectors

Phase 7: Cleanup

7.1 Unused Files (1,052 files)

Most are external dependencies in external/sogen/deps/:

  • Capstone test files (.s.cs files)
  • FlatBuffers test/sample files
  • Unicorn bindings samples

Action:

  1. Review if external/ needs to be in repo or should be submodule/package
  2. Add to .gitignore if generated
  3. Remove if truly unused

7.2 Code Markers Requiring Attention

Marker Count Priority
WARNING 31 High
TODO 9 Medium
WORKAROUND 1 Low

High-Priority Warnings:

  • ApiEmulator.EmulateIsDebuggerPresent (src:1175)
  • ApiEmulator.EmulateMemcpy (src:1626)
  • VmExecutor.HandleDebugBreak (src:1399)
  • ProcessEnvironment.InitializePeb64 (src:61)

Implementation Roadmap

Sprint 1: Security Fixes (Week 1-2)

  • Implement path canonicalization helpers
  • Fix all 59 path traversal vulnerabilities
  • Add input sanitization for logging

Sprint 2: God Class Extraction (Week 3-4)

  • Extract ApiEmulator into 4+ smaller classes
  • Create interfaces for dependency injection
  • Add unit tests for extracted classes

Sprint 3: Long Method Refactoring (Week 5-6)

  • Refactor RealModeExecutor.ExecuteOpcode
  • Refactor ApiEmulator.EmulateCall
  • Implement dispatch tables/strategy patterns

Sprint 4: Performance & Duplication (Week 7-8)

  • Fix LINQ in loops
  • Replace string concatenation with StringBuilder
  • Consolidate duplicate API stubs

Sprint 5: Logging & Cleanup (Week 9-10)

  • Add logging framework integration
  • Instrument critical paths
  • Remove/reorganize unused files

Metrics Targets

Metric Current Target
Health Score 0 70+
Max Cyclomatic Complexity 297 <50
Methods >200 lines 15 0
God Classes (>50 methods) 10 0
Security Vulnerabilities 79 0
Technical Debt 239h <80h