Updating the interactive journal app #177
Conversation
|
Semgrep found 7
Detected a logger that logs user input without properly neutralizing the output. The log message could contain characters like View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>apps/project-assistant/backend/app/routers/routes.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/2157af73c29399a358151fe4e6562232ced16e12/apps/project-assistant/backend/app/routers/routes.py#L214 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 214] project_id</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/2157af73c29399a358151fe4e6562232ced16e12/apps/project-assistant/backend/app/routers/routes.py#L214 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 214] project_id</a>"]
end
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/2157af73c29399a358151fe4e6562232ced16e12/apps/project-assistant/backend/app/routers/routes.py#L220 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 220] f"Deleted project {project_id}: "<br> f"{messages.deleted_count} messages, {memories.deleted_count} memories"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
🛟 Help? Slack #semgrep-help or go/semgrep-help. Resolution Options:
|
![semgrep-code-mongodb[bot]](https://avatars.githubusercontent.com/u/45120?s=60&v=4){kind=small}
| db.projects.delete_one({"_id": ObjectId(project_id)}) | ||
| messages = db.messages.delete_many({"project_id": project_id}) | ||
| todos = db.memories.delete_many({"project_id": project_id, "type": "todo"}) | ||
| logger.info(f"Deleted project {project_id}: {messages.deleted_count} messages, {todos.deleted_count} todos") |
There was a problem hiding this comment.
Detected a logger that logs user input without properly neutralizing the output. The log message could contain characters like and and cause an attacker to forge log entries or include malicious content into the logs. Use proper input validation and/or output encoding to prevent log entries from being forged.
🚀 Fixed in commit 4997b17 🚀
| db = get_database() | ||
| db.projects.delete_one({"_id": ObjectId(project_id)}) | ||
| messages = db.messages.delete_many({"project_id": project_id}) | ||
| logger.info(f"Deleted project {project_id}: {messages.deleted_count} messages") |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
User-controlled project_id from the path is logged directly, enabling log forging via newline/control-character injection.
More details about this
logger.info(f"Deleted project {project_id}: {messages.deleted_count} messages") logs the URL path parameter project_id directly. In this FastAPI route, project_id comes from the path /{project_id}, so an attacker controls its value.
Exploit scenario:
- Step 1: Send a crafted DELETE request like DELETE /projects/%0AINFO%20admin%20logged%20in%3A%20true where %0A is a newline. FastAPI passes the decoded string ("\nINFO admin logged in: true") into delete_project(project_id: str).
- Step 2: The f-string writes it verbatim into the log: "Deleted project \nINFO admin logged in: true: 42 messages".
- Step 3: The newline splits the log, creating a forged INFO entry that appears to come from the application, polluting audits and alerting. Variants using \r or log-like prefixes can insert fake WARN/ERROR lines or manipulate downstream SIEM parsers.
Because project_id is user-controlled and interpolated without neutralization, an attacker can inject newlines or control characters to forge or confuse log entries.
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>apps/project-assistant/backend/app/routers/routes.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/4997b17b2950c4de4e743ada303e58e37f893f90/apps/project-assistant/backend/app/routers/routes.py#L184 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 184] project_id</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/4997b17b2950c4de4e743ada303e58e37f893f90/apps/project-assistant/backend/app/routers/routes.py#L184 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 184] project_id</a>"]
end
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/mongodb-developer/GenAI-Showcase/blob/4997b17b2950c4de4e743ada303e58e37f893f90/apps/project-assistant/backend/app/routers/routes.py#L188 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 188] f"Deleted project {project_id}: {messages.deleted_count} messages"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
✨ Commit Assistant fix suggestion
| logger.info(f"Deleted project {project_id}: {messages.deleted_count} messages") | |
| # Use logger argument interpolation to avoid log injection via untrusted user input. | |
| logger.info("Deleted project %r: %d messages", project_id, messages.deleted_count) |
View step-by-step instructions
- Update the logger statement to avoid logging untrusted user input (such as
project_id) directly. - Validate or sanitize
project_idbefore including it in the log. If it is expected to be a valid ObjectId, validate it first withbson.ObjectId.is_valid(project_id)and log a generic message if not valid. - Alternatively, if you need to log the
project_idfor debugging, encode or escape it to prevent special characters from affecting the logs. You can do this withrepr(project_id)or similar:logger.info(f"Deleted project %r: %d messages", project_id, messages.deleted_count). - Prefer using logger argument interpolation instead of f-strings to protect against log injection:
logger.info("Deleted project %r: %d messages", project_id, messages.deleted_count). This prevents special characters inproject_idfrom being interpreted as part of the log structure.
Using logger argument interpolation ensures any untrusted data is safely handled by the logger and can't manipulate log formatting.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by tainted-log-injection-stdlib-fastapi.
🛟 Help? Slack #semgrep-help or go/semgrep-help.
Resolution Options:
- Fix the code
- Reply
/fp $reason(if security gap doesn’t exist) - Reply
/ar $reason(if gap is valid but intentional; add mitigations/monitoring) - Reply
/other $reason(e.g., test-only)
You can view more details about this finding in the Semgrep AppSec Platform.
No description provided.