-
Notifications
You must be signed in to change notification settings - Fork 184
pySCG: adding 117 as part of #531 #996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: emcdtho <[email protected]>
…e, related guidelines Signed-off-by: Helge Wehder <[email protected]>
I have fixed some of the formatting and cosmetics but am not sure what to do about that Related guidelines, References puzzle. |
# 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. | ||
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. |
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 \r
but 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
``` | ||
|
||
The output demonstrates successful log injection: | ||
**Expample output of noncompliant01.py:** |
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.
Typo: should be 'Example', but I think the previous test was clearer :-)
Also, 'Example' is slightly misleading; it's not an example of possible output, it's the only output that the code should produce.
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.
I was following our template. I can change it back to
Output of compliant01.py:
Point is to follow a common structure without any interpretation of what the thing does in the title.
<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> |
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.
I don't think this was referenced?
and i think we need the main references like CWE-117 in the Bibliography and not just in Releated Guidelines.
Ideally though the bibliography should be auto generated.
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.
Copy and paste error I am going to correct.
Regards 'automation', we have an open issue:
We may also adopt an alternative more machine readable way of referencing related guidelines as part of our restructuring effort's.
For the time being we continue to follow what we agreed on as per template:
https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Secure-Coding-Guide-for-Python/templates/README_TEMPLATE.md
Reverting changes while keeping cosmetics. Related guidelines and References are now back to having the same content with nested linkage in the related guidelines. Signed-off-by: myteron <[email protected]>
Signed-off-by: emcdtho <[email protected]>
|
||
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** [cwe117]. 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** [cwe93] weakness. Attackers exploit this using the **CAPEC-93: Log Injection-Tampering-Forging** [capec93] attack pattern. |
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.
this type of relation is already covered (but not explained) in 'related guidelines'. It gets difficult to know what to list as reference vs related guidelines if guidelines are this heavily listed as an argument in the body. We typically do not explain these relations unless the explianation is required to understand the content of the rule.
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.
Agree, my thinking was that because this this is so similar to CWE-93, and because the example of CRLF in logs is actually the second example for https://cwe.mitre.org/data/definitions/93.html (so the example given here also covers CWE-93) that it was worth clarifying the relationship. I don't object to removing the second paragraph if you'd prefer that.
``` | ||
|
||
The output demonstrates successful log injection: | ||
**Expample output of noncompliant01.py:** |
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.
I was following our template. I can change it back to
Output of compliant01.py:
Point is to follow a common structure without any interpretation of what the thing does in the title.
<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> |
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.
Copy and paste error I am going to correct.
Regards 'automation', we have an open issue:
We may also adopt an alternative more machine readable way of referencing related guidelines as part of our restructuring effort's.
For the time being we continue to follow what we agreed on as per template:
https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Secure-Coding-Guide-for-Python/templates/README_TEMPLATE.md
# 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. | ||
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. |
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
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.
The rule looks good. I left a suggestion about the introductory sentence that shows up in search engines.
@@ -0,0 +1,199 @@ | |||
# 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. |
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.
I think the first sentence here works as the introductory sentence in the first paragraph.
Signed-off-by: emcdtho <[email protected]>
@@ -0,0 +1,199 @@ | |||
# CWE-117: Improper Output Neutralization for Logs | |||
|
|||
Ensure all untrusted data is properly neutralized or sanitized before writing to application 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. |
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.
This new sentence works, too. Just make sure it's in a separate paragraph:
Ensure all untrusted data is properly neutralized or sanitized before writing to application 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. | |
Ensure all untrusted data is properly neutralized or sanitized before writing to application 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. |
Documentation and code samples for CWE-117 (Improper Output Neutralization for Logs) in the Python Secure Coding Guide.