This repository was archived by the owner on Jan 5, 2023. It is now read-only.
Improvements to clear-text logging query#243
Merged
max-schaefer merged 10 commits intogithub:masterfrom Jul 7, 2020
Merged
Conversation
added 10 commits
June 30, 2020 10:05
…t.Stringer.String()`. These two are very heavily overloaded and cause all sorts of false positives.
smowton
approved these changes
Jul 7, 2020
owen-mc
approved these changes
Jul 7, 2020
Contributor
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
The changes to CleartextLogging.expected in "Teach CleartextLogging not to track through" should be in the following commit, "Make clear-text logging sources more precise."
ceh
pushed a commit
to ceh-forks/codeql-go
that referenced
this pull request
Jul 22, 2020
IncompleteHostnameRegexp: Use a reluctant regexp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The overall aim of this PR is twofold: (1) flag CVE-2019-11250, (2) without introducing lots of false positives.
The first two commits are all it takes to achieve (1): extend our model of the
gloglogging library to also include a fork that Kubernetes uses, and recognise HTTP-request headers (in particularAuthorization) as a source of confidential data.All the other commits are about achieving (2); see individual commit messages for explanations.
On the whole, the results are good (internal link): we flag the CVE in the vulnerable version, and don't flag it in the fixed version. We also flag the same CVE in two other old versions of Kubernetes that we have in our benchmark suite.
Additionally, we lose a large number of alerts. I have looked through most of them, and they all looked like false positives, many of them involving spurious flow through imprecisely resolved calls to
StringorError, and a few where we consider the first argument ofFprintfand its cousins as a sink, which makes very little sense.There were a few mild performance regressions at first, but rerunning made them go away.