BUG: LocalClassSource path traversal check breaks inner classes

[sfloess]

Severity: BUG

File: LocalClassSource.java

Problem

The path traversal validation in getClassFilePath() is overly restrictive and breaks loading of inner classes, which use the $ character in their names.

Bug Details

Lines 63-65: Blocks "/" and "\" characters

if (className.contains("..") || className.contains("/") || className.contains("\\")) {
    throw new IOException("Invalid class name (potential path traversal): " + className);
}

Problem: This check is AFTER ClassNameUtil.toClassFilePath() should be called, but className still has dots at this point.

Wait, looking closer: The check happens BEFORE toClassFilePath() (line 67), so:

• className = "com.example.MyClass" (dots, not slashes)
• Check for "/" and "\" will never match because className hasn't been converted yet
• So the check is POINTLESS - it never triggers

Actual flow:

1. Line 63: Check if className contains "/" or "\" - will never match
2. Line 67: Convert dots to slashes: "com.example.MyClass" → "com/example/MyClass.class"
3. Line 68: Resolve against basePath
4. Lines 71-72: Check if resolved path starts with basePath

The check on lines 63-65 is REDUNDANT because:

• The real protection is lines 71-72 (startsWith check)
• Lines 63-65 check the wrong format (before conversion)

However, there IS a real issue with size limits

Lines 40-41:

if (Files.exists(classFile) && Files.isRegularFile(classFile)) {
    return Files.readAllBytes(classFile);
}

Problem: Files.readAllBytes() can throw OutOfMemoryError on very large files

Scenario:

1. Developer accidentally puts a 5GB log file in the classes directory
2. File is named something.class
3. JClassLoader tries to load it
4. Files.readAllBytes() tries to allocate 5GB array
5. OutOfMemoryError

Fix: Add size validation:

@Override
public byte[] loadClassData(String className) throws IOException {
    Path classFile = getClassFilePath(className);
    
    if (!Files.exists(classFile)) {
        throw new IOException("Class file not found: " + classFile);
    }
    
    if (!Files.isRegularFile(classFile)) {
        throw new IOException("Not a regular file: " + classFile);
    }
    
    // Check size before reading
    long size = Files.size(classFile);
    if (size > MAX_CLASS_SIZE) {  // e.g., 10MB
        throw new IOException(
            "Class file too large: " + size + " bytes (max " + MAX_CLASS_SIZE + ")"
        );
    }
    
    if (size > Integer.MAX_VALUE) {
        throw new IOException(
            "Class file exceeds Java array limit: " + size + " bytes"
        );
    }
    
    return Files.readAllBytes(classFile);
}

Add MAX_CLASS_SIZE constant:

private static final long MAX_CLASS_SIZE = 10 * 1024 * 1024;  // 10MB

// Or make it configurable via constructor:
private final long maxClassSize;

public LocalClassSource(Path basePath, long maxClassSize) {
    this.basePath = Objects.requireNonNull(basePath, "basePath cannot be null")
        .toAbsolutePath()
        .normalize();
    this.maxClassSize = maxClassSize;
}

public LocalClassSource(Path basePath) {
    this(basePath, MAX_CLASS_SIZE);
}

Recommendations

1. Remove lines 63-65 - they're redundant and check the wrong format:

   private Path getClassFilePath(String className) throws IOException {
       // Removed redundant check
       
       String fileName = ClassNameUtil.toClassFilePath(className);
       Path resolvedPath = basePath.resolve(fileName).normalize();
   
       // This is the REAL protection
       if (!resolvedPath.startsWith(basePath)) {
           throw new IOException("Path traversal attempt detected: " + className);
       }
   
       return resolvedPath;
   }

2. Add size validation to prevent OOM on large files

3. Improve error messages - separate "not found" from "not a file"

Impact

Current issues:

• OOM risk: Large files can crash JVM
• Redundant code: Lines 63-65 provide no actual protection
• Confusing: Path traversal check is in wrong place

With fixes:

• Bounded memory: Size limits prevent OOM
• Cleaner code: Remove redundant checks
• Better errors: Clear messages for each failure mode

Related Issues

• Issue CRITICAL: MinioClassSource has 7+ production-breaking issues #51: MinioClassSource OOM vulnerability
• Issue CRITICAL: HdfsClassSource has no size limits - OOM vulnerability #52: HdfsClassSource OOM vulnerability
• Issue CRITICAL: RestApiClassSource has 6+ serious bugs #53: RestApiClassSource OOM vulnerability
• Issue MAJOR: JarRemoteClassSource downloads multi-GB JARs with no size limits #54: JarRemoteClassSource size limits

[sfloess]

Fixed by commit 584136e. Added size validation (10MB default, configurable) to prevent OOM on large files, removed redundant path traversal check, and improved error messages.