Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Treat zap custom encoders as sanitizers for log-injection checks#20912

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
danielriddell21 wants to merge8 commits intogithub:main
base:main
Choose a base branch
Loading
fromdanielriddell21:feature/zap-encoder-sanitizer

Conversation

@danielriddell21
Copy link

@danielriddell21danielriddell21 commentedNov 25, 2025•
edited
Loading

Summary

Add an experimental CodeQL helper and query to treat custom zap encoders (types implementing go.uber.org/zap/zapcore.Encoder) as sanitizers for the purposes of log-injection detection. This reduces false positives where applications use a custom encoder to escape or sanitize log field values.

Notes for reviewers

  • This change is non-invasive: it adds an experimental suppression query rather than modifying built-in log-injection rules directly.

Risks

  • If a type implements zapcore.Encoder but does not actually sanitize input, this change could suppress a genuine finding. Use the whitelist to exclude such types.

@danielriddell21danielriddell21 requested a review froma team as acode ownerNovember 25, 2025 18:11
@danielriddell21danielriddell21 marked this pull request as draftNovember 25, 2025 18:33
@danielriddell21danielriddell21 changed the titleFeature/zap encoder sanitizerTreat zap custom encoders as sanitizers for log-injection checksNov 25, 2025
@danielriddell21danielriddell21force-pushed thefeature/zap-encoder-sanitizer branch fromd9379df to360014fCompareNovember 25, 2025 19:18
@danielriddell21danielriddell21 marked this pull request as ready for reviewNovember 25, 2025 19:19
@owen-mc
Copy link
Contributor

This PR doesn't make much sense. I don't think the tests would pass.

There is already a query calledgo/log-injection. It is located atgo/ql/src/Security/CWE-117/LogInjection.ql. All sanitizers for it are ingo/ql/lib/semmle/go/security/LogInjectionCustomizations.qll. Tests for it are atgo/ql/test/query-tests/Security/CWE-117/LogInjection.go. Note that we already include variations onstrings.ReplaceAll(s, "\n", "X") as a sanitizer.

@danielriddell21danielriddell21 marked this pull request as draftNovember 25, 2025 20:05
@danielriddell21
Copy link
Author

danielriddell21 commentedNov 25, 2025•
edited
Loading

This PR doesn't make much sense. I don't think the tests would pass.

There is already a query calledgo/log-injection. It is located atgo/ql/src/Security/CWE-117/LogInjection.ql. All sanitizers for it are ingo/ql/lib/semmle/go/security/LogInjectionCustomizations.qll. Tests for it are atgo/ql/test/query-tests/Security/CWE-117/LogInjection.go. Note that we already include variations onstrings.ReplaceAll(s, "\n", "X") as a sanitizer.

Hi@owen-mc thanks for looking. I am pretty new to codeql.

So I will try and move all the stuff into the correct place when its ready. Am I okay to tag you when It is ready for a review?

@danielriddell21danielriddell21force-pushed thefeature/zap-encoder-sanitizer branch from0aea3eb to997d300CompareNovember 25, 2025 20:33
@danielriddell21
Copy link
Author

My aim is to allow using a zap encoder with a sanitise within as a valid way to suppress a CWE 117

@owen-mc
Copy link
Contributor

Things are in the right place now, but the tests still don't make any sense. May I ask, are you using an LLM coding assistant? Yes, please tag me when you have got the tests passing locally on your machine, or you are stuck and need help with the CodeQL.

privatepredicateisSafeZapEncoder(Typet){
exists(TypezapEncoder|
// Matches go.uber.org/zap/zapcore.JSONEncoder
zapEncoder.hasQualifiedName("go.uber.org/zap/zapcore","JSONEncoder")and
Copy link
Contributor

Choose a reason for hiding this comment

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

This type doesn't exist, at least according tohttps://pkg.go.dev/go.uber.org/zap/zapcore.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@owen-mcowen-mcowen-mc left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@danielriddell21@owen-mc
[8]ページ先頭
©2009-2025 Movatter.jp