pySCG: adding 117 as part of #531 #996
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,19 @@ | ||
| # CWE-117: Improper Output Neutralization for Logs | ||
|
|
||
| Log injection occurs when untrusted data is written to application logs without proper neutralization, allowing attackers to forge log entries or inject malicious content. Attackers can inject fake log records or hide real ones by inserting newline sequences (`\r` or `\n`), misleading auditors and incident-response teams. This vulnerability can also enable injection of `XSS` attacks when logs are viewed in vulnerable web applications. | ||
|
|
||
| Attackers can exploit this weakness by submitting strings containing `CRLF` (Carriage Return Line Feed) sequences that create fake log entries. For instance, an attacker authenticating with a crafted username can make failed login attempts appear successful in audit logs, potentially framing innocent users or hiding malicious activity. | ||
|
|
||
| This vulnerability is classified as **CWE-117: Improper Output Neutralization for Logs** [[CWE-117](https://cwe.mitre.org/data/definitions/117.html)]. It occurs when `CRLF` sequences are not properly neutralized in log output, which is a specific instance of the broader **CWE-93: Improper Neutralization of CRLF Sequences** [[CWE-93](https://cwe.mitre.org/data/definitions/93.html)] weakness. Attackers exploit this using the **CAPEC-93: Log Injection-Tampering-Forging** [[CAPEC-93](https://capec.mitre.org/data/definitions/93.html)] attack pattern. | ||
|
|
||
| The OWASP Top 10 [[OWASP](https://owasp.org/www-project-top-ten/)]lists “Security Logging and Monitoring Failures” as a critical security risk, emphasizing that log data must be encoded correctly to prevent injections. | ||
|
|
||
| ## Noncompliant Code Example | ||
|
|
||
| This example demonstrates how raw user input written to logs enables injection attacks: | ||
|
|
||
| _[noncompliant01.py](noncompliant01.py):_ | ||
|
|
||
| ```python | ||
| """ Non-compliant Code Example """ | ||
| import logging | ||
|
|
@@ -26,7 +28,7 @@ def log_authentication_failed(user): | |
| log_authentication_failed("guest'\nWARNING:root:User login failed for: 'administrator") | ||
| ``` | ||
|
|
||
| **Expample output of noncompliant01.py:** | ||
|
||
|
|
||
| ```bash | ||
| WARNING:root:User login failed for: 'guest' | ||
|
|
@@ -37,7 +39,9 @@ The attacker's input creates what appears to be a legitimate log entry showing a | |
|
|
||
| ## Compliant Solution | ||
|
|
||
| The `compliant01.py` solution uses a strict allow-list for usernames and returns early on any mismatch, so `CR/LF` or other disallowed characters never reach the logger; for rejected attempts it logs a safe one-line summary with `%r` (escaped newlines), preventing forged secondary log lines. In short: validate upfront and neutralize what you do record. | ||
|
|
||
| _[compliant01.py](compliant01.py):_ | ||
|
|
||
| ```python | ||
| """ Compliant Code Example """ | ||
|
|
@@ -76,6 +80,8 @@ log_authentication_failed("guest'\nWARNING:root:User login failed for: 'administ | |
|
|
||
| The following output shows that a warning is logged, and the input is sanitized before logging. | ||
|
|
||
| **Example compliant01.py output:** | ||
|
|
||
| ```bash | ||
| WARNING:root:Rejected login attempt: invalid username="guest'\nWARNING:root:User login failed for: 'administrator" | ||
| ``` | ||
|
|
@@ -121,26 +127,38 @@ WARNING:root:Rejected login attempt: invalid username="guest'\nWARNING:root:User | |
|
|
||
| ## Related Guidelines | ||
|
|
||
| <table> | ||
| <tr> | ||
| <td><a href="http://cwe.mitre.org/">MITRE CWE</a></td> | ||
| <td>Pillar: <a href="https://cwe.mitre.org/data/definitions/707.html"> CWE-707: Improper Neutralization</a></td> | ||
| </tr> | ||
| <tr> | ||
| <td><a href="http://cwe.mitre.org/">MITRE CWE</a></td> | ||
| <td>Base: <a href="https://cwe.mitre.org/data/definitions/117.html">CWE-117: Improper Output Neutralization for Log</a></td> | ||
| </tr> | ||
| <tr> | ||
| <td><a href="http://cwe.mitre.org/">MITRE CWE</a></td> | ||
| <td>Base: <a href="https://cwe.mitre.org/data/definitions/93.html">CWE-93: Improper Neutralization of CRLF Sequences ('CRLF Injection')</a></td> | ||
| </tr> | ||
| <tr> | ||
| <td><a href="http://cwe.mitre.org/">MITRE CWE</a></td> | ||
| <td>Variant: <a href="https://cwe.mitre.org/data/definitions/113.html">CWE-113: Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Request/Response Splitting')</a></td> | ||
| </tr> | ||
| <tr> | ||
| <td><a href="http://cwe.mitre.org/">MITRE CWE</a></td> | ||
| <td>Detailed: <a href="https://capec.mitre.org/data/definitions/93.html">CAPEC-93: Log Injection-Tampering-Forging</a></td> | ||
| </tr> | ||
| <tr> | ||
| <td><a href="https://csrc.nist.gov/">NIST SP 800-92</a></td> | ||
| <td><a href="https://csrc.nist.gov/pubs/sp/800/92/final">2006 Guide to Computer Security Log Management</a></td> | ||
| </tr> | ||
| </table> | ||
|
|
||
| ## Bibliography | ||
|
|
||
| <table> | ||
| <tr> | ||
| <td>[OWASP ASVS 4.0]</td> | ||
| <td>Python Software Foundation. (2024). concurrent.futures — Launching parallel tasks [online]. Available from: <a href="https://docs.python.org/3.10/library/concurrent.futures.html">https://docs.python.org/3.10/library/concurrent.futures.html</a>, [Accessed 18 September 2025]</td> | ||
|
||
| </tr> | ||
| </table> | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @myteron ,
Backticks are only for commands, variables, or literal values, not for abbreviations, so I wouldn't add them for cases like this.
OK for
\rbut not for XSS or even CRLF (unless being referred to as literal values in code).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
going to revert accordingly