Go: Add Tainted Path sanitizers#17759
Conversation
owen-mc
left a comment
There was a problem hiding this comment.
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.
owen-mc
left a comment
There was a problem hiding this comment.
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
Show resolved
Hide resolved
owen-mc
left a comment
There was a problem hiding this comment.
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 <62447351+owen-mc@users.noreply.github.com>
|
committing suggestions didn't work 😞 |
Shows how much I know 😆 . |
|
@owen-mc Just wanted to remind to merge since its been a couple months. |
go/ql/test/query-tests/Security/CWE-022/GorillaMuxDefault/TaintedPath.qlref
Outdated
Show resolved
Hide resolved
go/ql/test/query-tests/Security/CWE-022/GorillaMuxSkipClean/TaintedPath.qlref
Outdated
Show resolved
Hide resolved
|
Oh no! Sorry about that, it slipped off my list. Thanks for prodding me. Also, I forget that other people can't press the merge button 😆 . The postprocess queries that you're using were moved around so I've fixed the references, which should make CI pass. |
Add gorilla mux.Vars sanitizer