Skip to content

fix: sanitize user-supplied command before logging to prevent log injection#1671

Open
rajkumar-prog wants to merge 2 commits into
openai:mainfrom
rajkumar-prog:fix/log-injection-flask-playwright
Open

fix: sanitize user-supplied command before logging to prevent log injection#1671
rajkumar-prog wants to merge 2 commits into
openai:mainfrom
rajkumar-prog:fix/log-injection-flask-playwright

Conversation

@rajkumar-prog
Copy link
Copy Markdown

Problem

Fixes #1542

In _execute_command(), the command field from the request body is written directly to logs without sanitization:

logger.info(f"Error executing command: {command}")

Because command is user-controlled, an attacker can embed newlines (\n, \r) to inject fake log entries — e.g.:

page.goto("legit.com")
2024-01-01 00:00:00 INFO: Session started as root

The same unsanitized string is also passed as ValueError.args[0], which callers log via logger.error(e.args[0]).

Fix

Strip newlines and carriage returns from command before any logging by creating a safe_command local variable:

safe_command = command.replace("\n", "\\n").replace("\r", "\\r")
logger.info(f"Error executing command: {safe_command}")
...
raise ValueError(
    f"Error executing command {safe_command}",
    ...
)

The original command (unsanitized) is still forwarded to the API error response unchanged, so callers retain full fidelity — only the log output is sanitized.

Change

evals/elsuite/multistep_web_tasks/docker/flask-playwright/app.py — 2 logging sites updated (+2 lines, -1 line net)

rajkumar-prog and others added 2 commits May 16, 2026 14:09
…ection

Fixes openai#1542

Newlines and carriage returns in user-controlled `command` input were
written to logs verbatim, allowing an attacker to inject fake log entries.
Replace \n and \r with their escaped equivalents before logging, and also
sanitize the ValueError message arg (which callers pass to logger.error).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log injection alert

1 participant