Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* More logging functions are now recognized as not returning or panicking.
31 changes: 20 additions & 11 deletions go/ql/lib/semmle/go/Concepts.qll
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,13 @@ private class ExternalLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
}
}

/**
* A call to an interface that looks like a logger. It is common to use a
* locally-defined interface for logging to make it easy to changing logging
* library.
*/
private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
HeuristicLoggerCall() {
exists(Method m, string tp, string logFunctionPrefix, string name |
m = this.getTarget() and
m.hasQualifiedName(_, tp, name) and
m.getReceiverBaseType().getUnderlyingType() instanceof InterfaceType
private class HeuristicLoggerFunction extends Method {
string logFunctionPrefix;

HeuristicLoggerFunction() {
exists(string tp, string name |
this.hasQualifiedName(_, tp, name) and
this.getReceiverBaseType().getUnderlyingType() instanceof InterfaceType
|
tp.regexpMatch(".*[lL]ogger") and
logFunctionPrefix =
Expand All @@ -435,6 +431,19 @@ private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode
)
}

override predicate mayReturnNormally() { logFunctionPrefix != "Fatal" }

override predicate mustPanic() { logFunctionPrefix = "Panic" }
}

/**
* A call to an interface that looks like a logger. It is common to use a
* locally-defined interface for logging to make it easy to change logging
* library.
Comment thread
owen-mc marked this conversation as resolved.
*/
private class HeuristicLoggerCall extends LoggerCall::Range, DataFlow::CallNode {
HeuristicLoggerCall() { this.getTarget() instanceof HeuristicLoggerFunction }

override DataFlow::Node getAMessageComponent() { result = this.getASyntacticArgument() }
}

Expand Down
35 changes: 30 additions & 5 deletions go/ql/lib/semmle/go/frameworks/Glog.qll
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,37 @@ import go
* forks.
*/
module Glog {
/** Gets a package name for `glog` or `klog` (which is a fork). */
string packagePath() {
result =
package([
"github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog", "github.com/barakmich/glog"
], "")
}

private class GlogFunction extends Function {
int firstPrintedArg;
string format;
string level;

GlogFunction() {
exists(string pkg, string fn, string level |
pkg = package(["github.com/golang/glog", "gopkg.in/glog", "k8s.io/klog"], "") and
exists(string pkg, string context, int nContextArgs, string depth, int nDepthArgs, string fn |
pkg = packagePath() and
level = ["Error", "Exit", "Fatal", "Info", "Warning"] and
(
fn = level + ["", "f", "ln"] and firstPrintedArg = 0
context = "" and nContextArgs = 0
or
context = "Context" and nContextArgs = 1
) and
(
depth = "" and nDepthArgs = 0
or
fn = level + "Depth" and firstPrintedArg = 1
depth = "Depth" and nDepthArgs = 1
) and
format = ["", "f", "ln"] and
(
fn = level + context + depth + format and
firstPrintedArg = nContextArgs + nDepthArgs
)
|
this.hasQualifiedName(pkg, fn)
Expand All @@ -35,10 +55,15 @@ module Glog {
* Gets the index of the first argument that may be output, including a format string if one is present.
*/
int getFirstPrintedArg() { result = firstPrintedArg }

/** Holds if this function takes a format string. */
predicate formatter() { format = "f" }

override predicate mayReturnNormally() { level != "Fatal" and level != "Exit" }
}

private class StringFormatter extends StringOps::Formatting::Range instanceof GlogFunction {
StringFormatter() { this.getName().matches("%f") }
StringFormatter() { this.formatter() }

override int getFormatStringIndex() { result = super.getFirstPrintedArg() }
}
Expand Down
6 changes: 6 additions & 0 deletions go/ql/lib/semmle/go/frameworks/Logrus.qll
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ module Logrus {
this.(Method).hasQualifiedName(packagePath(), ["Entry", "Logger"], name)
)
}

override predicate mayReturnNormally() {
not exists(string level, string suffix | level = ["Fatal", "Panic"] |
this.getName() = level + suffix
)
}
}

private class StringFormatters extends StringOps::Formatting::Range instanceof LogFunction {
Expand Down
4 changes: 2 additions & 2 deletions go/ql/lib/semmle/go/frameworks/Zap.qll
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module Zap {
}

/** A Zap logging function which always panics. */
private class FatalLogMethod extends Method {
private class FatalLogMethod extends ZapFunction {
FatalLogMethod() {
this.hasQualifiedName(packagePath(), "Logger", "Fatal")
or
Expand All @@ -58,7 +58,7 @@ module Zap {
}

/** A Zap logging function which always panics. */
private class MustPanicLogMethod extends Method {
private class MustPanicLogMethod extends ZapFunction {
MustPanicLogMethod() {
this.hasQualifiedName(packagePath(), "Logger", "Panic")
or
Expand Down
47 changes: 21 additions & 26 deletions go/ql/lib/semmle/go/frameworks/stdlib/Log.qll
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,37 @@ module Log {
}

private class LogFormatter extends StringOps::Formatting::Range instanceof LogFunction {
LogFormatter() { this.getName() = ["Fatalf", "Panicf", "Printf"] }
LogFormatter() { this.getName() = ["Fatalf", "Panicf", "Printf", "Panic", "Panicf", "Panicln"] }

override int getFormatStringIndex() { result = 0 }
}

/** A fatal log function, which calls `os.Exit`. */
private class FatalLogFunction extends Function {
FatalLogFunction() { this.hasQualifiedName("log", ["Fatal", "Fatalf", "Fatalln"]) }
FatalLogFunction() {
exists(string fn | fn = ["Fatal", "Fatalf", "Fatalln"] |
this.hasQualifiedName("log", fn)
or
this.(Method).hasQualifiedName("log", "Logger", fn)
)
}

override predicate mayReturnNormally() { none() }
}

/** A log function which must panic. */
private class PanicLogFunction extends Function {
PanicLogFunction() {
exists(string fn | fn = ["Panic", "Panicf", "Panicln"] |
this.hasQualifiedName("log", fn)
or
this.(Method).hasQualifiedName("log", "Logger", fn)
)
}

override predicate mustPanic() { any() }
}

// These models are not implemented using Models-as-Data because they represent reverse flow.
private class FunctionModels extends TaintTracking::FunctionModel {
FunctionInput inp;
Expand All @@ -63,30 +82,6 @@ module Log {
FunctionOutput outp;

MethodModels() {
// signature: func (*Logger) Fatal(v ...interface{})
this.hasQualifiedName("log", "Logger", "Fatal") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Fatalf(format string, v ...interface{})
this.hasQualifiedName("log", "Logger", "Fatalf") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Fatalln(v ...interface{})
this.hasQualifiedName("log", "Logger", "Fatalln") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Panic(v ...interface{})
this.hasQualifiedName("log", "Logger", "Panic") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Panicf(format string, v ...interface{})
this.hasQualifiedName("log", "Logger", "Panicf") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Panicln(v ...interface{})
this.hasQualifiedName("log", "Logger", "Panicln") and
(inp.isParameter(_) and outp.isReceiver())
or
// signature: func (*Logger) Print(v ...interface{})
this.hasQualifiedName("log", "Logger", "Print") and
(inp.isParameter(_) and outp.isReceiver())
Expand Down
Loading
Loading