Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Go: Add Log Injection query (CWE-117) - #609

Merged
atorralba merged 5 commits into
mainfrom
atorralba/log-injection-query
Nov 24, 2021
Merged

Go: Add Log Injection query (CWE-117)#609
atorralba merged 5 commits into
mainfrom
atorralba/log-injection-query

Conversation

@atorralba

@atorralbaatorralba commented Nov 19, 2021

Copy link
Copy Markdown
Contributor

This PR adds a query for Log Injection (CWE-117).

Description

If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.

Forgery can occur if a user provides some input with characters that are interpreted when the log output is displayed. If the log is displayed as a plain text file, then new line characters can be used by a malicious user. If the log is displayed as HTML, then arbitrary HTML may be include to spoof log entries.

To consider

No sanitizers were added, following the same reasoning as in the Java version.
Post-hoc edit by @smowton: this query DOES sanitize on removal of linebreak characters

Evaluation

The query shows results in 4820 projects. A small sample of them were reviewed and deemed TPs.

References

https://www.owasp.org/index.php/Log_Injection

@atorralba
atorralba requested a review from a team as a code ownerNovember 19, 2021 16:55
@atorralba
atorralbaforce-pushed the atorralba/log-injection-query branch from 4f26661 to f2017b6CompareNovember 22, 2021 08:15
@owen-mc

Copy link
Copy Markdown
Contributor

It looks good to me. Can you explain what's going on with the stubs?

@atorralba

Copy link
Copy Markdown
ContributorAuthor

It looks good to me. Can you explain what's going on with the stubs?

First time using depstubber, sorry. I somehow pushed the generated stub.goand the actual, downloaded source code of each dependency 😅

@atorralbaatorralba added the ready-for-doc-review This PR requires and is ready for review from the GitHub CodeQL docs team. label Nov 24, 2021

@mattpollardmattpollard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a minor typo, but otherwise, this is good to go 🚀

Comment threadql/src/Security/CWE-117/LogInjection.qhelp Outdated
Co-authored-by: Matt Pollard <mattpollard@users.noreply.github.com>
@atorralba
atorralba merged commit 662f880 into mainNov 24, 2021
@atorralba
atorralba deleted the atorralba/log-injection-query branch November 24, 2021 14:52
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-for-doc-reviewThis PR requires and is ready for review from the GitHub CodeQL docs team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@atorralba@owen-mc@mattpollard@docubot