-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Go: Add Tainted Path sanitizers #17759
base: main
Are you sure you want to change the base?
Conversation
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.
Please add a test for net/url.URL.Path
, and mention it in the change note. Also, why do you think that reading that field is a sanitizer? I don't see anything in the docs about what characters it may or may not contain.
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.
Looks good - just need to add two tests for SkipClean (called with true and false).
go/ql/test/query-tests/Security/CWE-022/vendor/github.com/gorilla/mux/LICENSE
Outdated
Show resolved
Hide resolved
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.
Looks good! Only very minor comments left.
go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/MuxClean.go
Outdated
Show resolved
Hide resolved
go/ql/test/query-tests/Security/CWE-022/GorillaMuxSkipClean/MuxClean.go
Outdated
Show resolved
Hide resolved
I think if you're just committing my suggestions it doesn't dismiss my review. |
Co-authored-by: Owen Mansel-Chan <[email protected]>
committing suggestions didn't work 😞 |
Shows how much I know 😆 . |
Add gorilla mux.Vars sanitizer