mirror of
https://github.com/github/codeql.git
synced 2026-01-29 22:32:58 +01:00
Simplify Logrus model
Make methods which add data to entries sinks in their own right, rather than trying to track the data flow of the entry to a later logging call. This may cause some false positives, but only in the situation that tainted data is added to an entry and that entry is never logged. It will save us from false negatives when tainted data is added to an entry which flows across a function boundary to a logging call.
This commit is contained in:
@@ -15,33 +15,21 @@ module Logrus {
|
||||
result.matches(["Debug%", "Error%", "Fatal%", "Info%", "Panic%", "Print%", "Trace%", "Warn%"])
|
||||
}
|
||||
|
||||
bindingset[result]
|
||||
private string getAnEntryUpdatingMethodName() {
|
||||
result.regexpMatch("With(Context|Error|Fields?|Time)")
|
||||
}
|
||||
|
||||
private class LogCall extends LoggerCall::Range, DataFlow::CallNode {
|
||||
LogCall() { this.getTarget().hasQualifiedName(packagePath(), getALogResultName()) }
|
||||
LogCall() {
|
||||
this.getTarget().hasQualifiedName(packagePath(), getALogResultName()) or
|
||||
this.getTarget().(Method).hasQualifiedName(packagePath(), "Entry", getALogResultName()) or
|
||||
this.getTarget().hasQualifiedName(packagePath(), getAnEntryUpdatingMethodName()) or
|
||||
this.getTarget()
|
||||
.(Method)
|
||||
.hasQualifiedName(packagePath(), "Entry", getAnEntryUpdatingMethodName())
|
||||
}
|
||||
|
||||
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
|
||||
}
|
||||
|
||||
private class LogEntryCall extends LoggerCall::Range, DataFlow::MethodCallNode {
|
||||
LogEntryCall() {
|
||||
this.getTarget().(Method).hasQualifiedName(packagePath(), "Entry", getALogResultName())
|
||||
}
|
||||
|
||||
override DataFlow::Node getAMessageComponent() {
|
||||
result = this.getAnArgument()
|
||||
or
|
||||
exists(DataFlow::MethodCallNode addFieldCall, DataFlow::SsaNode entry |
|
||||
entry.getAUse() = this.getReceiver() and
|
||||
entry.getAUse() = addFieldCall.getReceiver()
|
||||
|
|
||||
addFieldCall.getCalleeName().regexpMatch("With(Context|Error|Fields?|Time)") and
|
||||
result = addFieldCall.getAnArgument()
|
||||
)
|
||||
or
|
||||
exists(DataFlow::CallNode entryBuild |
|
||||
entryBuild.getASuccessor*() = this.getReceiver() and
|
||||
entryBuild.getCalleeName().regexpMatch("With(Context|Error|Fields?|Time)") and
|
||||
result = entryBuild.getAnArgument()
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,11 +5,12 @@ package main
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/sirupsen/logrus"
|
||||
)
|
||||
|
||||
func logSomething(entry *logrus.Entry) {
|
||||
entry.Traceln(text) // $logger=text $f-:logger=fields
|
||||
entry.Traceln(text) // $logger=text
|
||||
}
|
||||
|
||||
func logrusCalls() {
|
||||
@@ -17,13 +18,13 @@ func logrusCalls() {
|
||||
var fields logrus.Fields = nil
|
||||
var fn logrus.LogFunction = nil
|
||||
var ctx context.Context
|
||||
tmp := logrus.WithContext(ctx) //
|
||||
tmp.Debugf(fmt, text) // $logger=ctx $logger=fmt $logger=text
|
||||
tmp = logrus.WithError(err) //
|
||||
tmp.Warn(text) // $logger=err $logger=text
|
||||
tmp = logrus.WithFields(fields) //
|
||||
tmp.Infoln(text) // $logger=fields $logger=text
|
||||
tmp = logrus.WithFields(fields) //
|
||||
tmp := logrus.WithContext(ctx) // $logger=ctx
|
||||
tmp.Debugf(fmt, text) // $logger=fmt $logger=text
|
||||
tmp = logrus.WithError(err) // $logger=err
|
||||
tmp.Warn(text) // $logger=text
|
||||
tmp = logrus.WithFields(fields) // $logger=fields
|
||||
tmp.Infoln(text) // $logger=text
|
||||
tmp = logrus.WithFields(fields) // $logger=fields
|
||||
logSomething(tmp)
|
||||
|
||||
logrus.Error(text) // $logger=text
|
||||
|
||||
Reference in New Issue
Block a user