-
Notifications
You must be signed in to change notification settings - Fork 2k
Go: Fix false positives when logging using %T
#19053
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
Changes from 11 commits
f173305
009e0e1
59d82b3
94c812c
646d28f
11ff0a0
bf78160
05a9480
f944ff4
bc40a42
da8ae84
662af6e
f677ddd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * False positives in "Log entries created from user input" (`go/log-injection`) and "Clear-text logging of sensitive information" (`go/clear-text-logging`) which correspond to the verb `%T` in a format specifier have been removed. There may also be more results in "Use of constant `state` value in OAuth 2.0 URL" (`go/constant-oauth2-state`). | ||
owen-mc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,13 @@ func glogTest() { | |
| glog.Warningf(fmt, text) // $ logger=fmt logger=text | ||
| glog.Warningln(text) // $ logger=text | ||
|
|
||
| // components corresponding to the format specifier "%T" are not considered vulnerable | ||
| glog.Errorf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| glog.Exitf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| glog.Fatalf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| glog.Infof("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| glog.Warningf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
|
||
|
|
||
| klog.Error(text) // $ logger=text | ||
| klog.ErrorDepth(0, text) // $ logger=text | ||
| klog.Errorf(fmt, text) // $ logger=fmt logger=text | ||
|
|
@@ -50,4 +57,11 @@ func glogTest() { | |
| klog.WarningDepth(0, text) // $ logger=text | ||
| klog.Warningf(fmt, text) // $ logger=fmt logger=text | ||
| klog.Warningln(text) // $ logger=text | ||
|
|
||
| // components corresponding to the format specifier "%T" are not considered vulnerable | ||
| klog.Errorf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| klog.Exitf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| klog.Fatalf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| klog.Infof("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| klog.Warningf("%s: found type %T", text, v) // $ logger="%s: found type %T" logger=text | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ package main | |
| const fmt = "formatted %s string" | ||
| const text = "test" | ||
|
|
||
| func main() { | ||
| var v []byte | ||
|
|
||
| func main() { | ||
| stdlib() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the motivation for this change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To remove linter warnings about lots of unused things 😆 . |
||
| } | ||
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.
Should this be less results, for the same reason?
Uh oh!
There was an error while loading. Please reload this page.
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.
In
go/constant-oauth2-state, arguments to loggercalls are used to rule out some alerts. So less arguments to loggercalls means more alerts, potentially. (In practice I doubt there will be any alert changes for that query.)