From 4b76966a49b7e600df2ce199c4f17c4ceeeaadca Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 12 Oct 2020 17:15:11 +0100 Subject: [PATCH 1/3] Model Spew logging framework --- ql/src/go.qll | 1 + ql/src/semmle/go/frameworks/Spew.qll | 39 ++++++++++++ .../go/frameworks/Spew/TaintFlows.expected | 17 ++++++ .../semmle/go/frameworks/Spew/TaintFlows.ql | 28 +++++++++ .../semmle/go/frameworks/Spew/go.mod | 8 +++ .../semmle/go/frameworks/Spew/test.go | 58 ++++++++++++++++++ .../github.com/davecgh/go-spew/spew/stub.go | 60 +++++++++++++++++++ .../go/frameworks/Spew/vendor/modules.txt | 6 ++ 8 files changed, 217 insertions(+) create mode 100644 ql/src/semmle/go/frameworks/Spew.qll create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.ql create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/go.mod create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/test.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/vendor/github.com/davecgh/go-spew/spew/stub.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/Spew/vendor/modules.txt diff --git a/ql/src/go.qll b/ql/src/go.qll index b52f07ef723..27916e2ee6b 100644 --- a/ql/src/go.qll +++ b/ql/src/go.qll @@ -37,6 +37,7 @@ import semmle.go.frameworks.Macaron import semmle.go.frameworks.Mux import semmle.go.frameworks.NoSQL import semmle.go.frameworks.Protobuf +import semmle.go.frameworks.Spew import semmle.go.frameworks.SQL import semmle.go.frameworks.Stdlib import semmle.go.frameworks.SystemCommandExecutors diff --git a/ql/src/semmle/go/frameworks/Spew.qll b/ql/src/semmle/go/frameworks/Spew.qll new file mode 100644 index 00000000000..eafe0694b0a --- /dev/null +++ b/ql/src/semmle/go/frameworks/Spew.qll @@ -0,0 +1,39 @@ +/** + * Provides models of commonly used functions in the `github.com/davecgh/go-spew/spew` package. + */ + +import go + +/** + * Provides models of commonly used functions in the `github.com/davecgh/go-spew/spew` package. + */ +module Spew { + private string packagePath() { result = "github.com/davecgh/go-spew/spew" } + + private class SpewCall extends LoggerCall::Range, DataFlow::CallNode { + int firstPrintedArg; + + SpewCall() { + exists(string fn | + fn in ["Dump", "Errorf", "Print", "Printf", "Println"] and firstPrintedArg = 0 + or + fn in ["Fdump", "Fprint", "Fprintf", "Fprintln"] and firstPrintedArg = 1 + | + this.getTarget().hasQualifiedName(packagePath(), fn) + ) + } + + override DataFlow::Node getAMessageComponent() { + result = this.getArgument(any(int i | i >= firstPrintedArg)) + } + } + + /** The `Sprint` function or one of its variants. */ + class Sprinter extends TaintTracking::FunctionModel { + Sprinter() { hasQualifiedName(packagePath(), ["Sdump", "Sprint", "Sprintln", "Sprintf"]) } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isParameter(_) and outp.isResult() + } + } +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.expected b/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.expected new file mode 100644 index 00000000000..f24231f8566 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.expected @@ -0,0 +1,17 @@ +| test.go:26:16:26:35 | call to getUntrustedString : string | test.go:33:14:33:23 | sUntrusted | +| test.go:26:16:26:35 | call to getUntrustedString : string | test.go:35:14:35:23 | sUntrusted | +| test.go:26:16:26:35 | call to getUntrustedString : string | test.go:41:18:41:27 | sUntrusted | +| test.go:26:16:26:35 | call to getUntrustedString : string | test.go:51:13:51:16 | str3 | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:30:12:30:21 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:31:13:31:22 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:32:15:32:24 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:34:17:34:26 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:36:17:36:26 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:38:16:38:25 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:39:17:39:26 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:40:19:40:28 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:42:21:42:30 | pUntrusted | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:45:13:45:16 | str1 | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:48:13:48:16 | str2 | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:54:13:54:16 | str4 | +| test.go:28:16:28:35 | call to getUntrustedStruct : Person | test.go:57:13:57:16 | str5 | diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.ql b/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.ql new file mode 100644 index 00000000000..fbd4149d6c9 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/TaintFlows.ql @@ -0,0 +1,28 @@ +import go + +class UntrustedFunction extends Function { + UntrustedFunction() { this.getName() = ["getUntrustedString", "getUntrustedStruct"] } +} + +class UntrustedSource extends DataFlow::Node, UntrustedFlowSource::Range { + UntrustedSource() { this = any(UntrustedFunction f).getACall() } +} + +class SinkFunction extends Function { + SinkFunction() { this.getName() = "sinkString" } +} + +class TestConfig extends TaintTracking::Configuration { + TestConfig() { this = "testconfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource } + + override predicate isSink(DataFlow::Node sink) { + sink = any(SinkFunction f).getACall().getAnArgument() or + sink = any(LoggerCall log).getAMessageComponent() + } +} + +from TaintTracking::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select source, sink diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/go.mod b/ql/test/library-tests/semmle/go/frameworks/Spew/go.mod new file mode 100644 index 00000000000..cf1e57ebd5c --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/go.mod @@ -0,0 +1,8 @@ +module codeql-go-tests/frameworks/Spew + +go 1.14 + +require ( + github.com/davecgh/go-spew v1.1.1 + github.com/github/depstubber v0.0.0-20200916130315-f3217697abd4 // indirect +) diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/test.go b/ql/test/library-tests/semmle/go/frameworks/Spew/test.go new file mode 100644 index 00000000000..513f1285b39 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/test.go @@ -0,0 +1,58 @@ +package main + +//go:generate depstubber -vendor github.com/davecgh/go-spew/spew "" Dump,Print,Println,Fdump,Fprint,Fprintln,Errorf,Fprintf,Printf,Sdump,Sprint,Sprintf,Sprintln + +import ( + "io" + + "github.com/davecgh/go-spew/spew" +) + +func main() {} + +type Person struct { + Name string + Address string +} + +func getUntrustedString() string { return "%v" } + +func getUntrustedStruct() Person { return Person{} } + +func sinkString(s string) {} + +func testSpew(w io.Writer) { + s := "%v" + sUntrusted := getUntrustedString() + p := Person{} + pUntrusted := getUntrustedStruct() + + spew.Dump(pUntrusted) // NOT OK + spew.Print(pUntrusted) // NOT OK + spew.Println(pUntrusted) // NOT OK + spew.Errorf(sUntrusted, p) // NOT OK + spew.Errorf(s, pUntrusted) // NOT OK + spew.Printf(sUntrusted, p) // NOT OK + spew.Printf(s, pUntrusted) // NOT OK + + spew.Fdump(w, pUntrusted) // NOT OK + spew.Fprint(w, pUntrusted) // NOT OK + spew.Fprintln(w, pUntrusted) // NOT OK + spew.Fprintf(w, sUntrusted, p) // NOT OK + spew.Fprintf(w, s, pUntrusted) // NOT OK + + str1 := spew.Sdump(pUntrusted) + sinkString(str1) // NOT OK + + str2 := spew.Sprint(pUntrusted) + sinkString(str2) // NOT OK + + str3 := spew.Sprintf(sUntrusted, p) + sinkString(str3) // NOT OK + + str4 := spew.Sprintf(s, pUntrusted) + sinkString(str4) // NOT OK + + str5 := spew.Sprintln(pUntrusted) + sinkString(str5) // NOT OK +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/github.com/davecgh/go-spew/spew/stub.go b/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/github.com/davecgh/go-spew/spew/stub.go new file mode 100644 index 00000000000..2e5adc02c43 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/github.com/davecgh/go-spew/spew/stub.go @@ -0,0 +1,60 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/davecgh/go-spew/spew, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/davecgh/go-spew/spew (exports: ; functions: Dump,Print,Println,Fdump,Fprint,Fprintln,Errorf,Fprintf,Printf,Sdump,Sprint,Sprintf,Sprintln) + +// Package spew is a stub of github.com/davecgh/go-spew/spew, generated by depstubber. +package spew + +import ( + io "io" +) + +func Dump(_ ...interface{}) {} + +func Errorf(_ string, _ ...interface{}) error { + return nil +} + +func Fdump(_ io.Writer, _ ...interface{}) {} + +func Fprint(_ io.Writer, _ ...interface{}) (int, error) { + return 0, nil +} + +func Fprintf(_ io.Writer, _ string, _ ...interface{}) (int, error) { + return 0, nil +} + +func Fprintln(_ io.Writer, _ ...interface{}) (int, error) { + return 0, nil +} + +func Print(_ ...interface{}) (int, error) { + return 0, nil +} + +func Printf(_ string, _ ...interface{}) (int, error) { + return 0, nil +} + +func Println(_ ...interface{}) (int, error) { + return 0, nil +} + +func Sdump(_ ...interface{}) string { + return "" +} + +func Sprint(_ ...interface{}) string { + return "" +} + +func Sprintf(_ string, _ ...interface{}) string { + return "" +} + +func Sprintln(_ ...interface{}) string { + return "" +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/modules.txt b/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/modules.txt new file mode 100644 index 00000000000..9e9ed5edf64 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Spew/vendor/modules.txt @@ -0,0 +1,6 @@ +# github.com/davecgh/go-spew v1.1.1 +## explicit +github.com/davecgh/go-spew +# github.com/github/depstubber v0.0.0-20200916130315-f3217697abd4 +## explicit +github.com/github/depstubber From 8811758e4425744b1b953ec8d5465df331e830ec Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 14 Oct 2020 14:49:50 +0100 Subject: [PATCH 2/3] Add change note --- change-notes/2020-10-14-spew.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 change-notes/2020-10-14-spew.md diff --git a/change-notes/2020-10-14-spew.md b/change-notes/2020-10-14-spew.md new file mode 100644 index 00000000000..46175b11f2a --- /dev/null +++ b/change-notes/2020-10-14-spew.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added support for the Spew deep pretty printing framework From b89775ac6531a5711eeea3bdf762c5b3c51be820 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Fri, 16 Oct 2020 10:56:27 +0100 Subject: [PATCH 3/3] Update change-notes/2020-10-14-spew.md Co-authored-by: Chris Smowton --- change-notes/2020-10-14-spew.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change-notes/2020-10-14-spew.md b/change-notes/2020-10-14-spew.md index 46175b11f2a..29103258c18 100644 --- a/change-notes/2020-10-14-spew.md +++ b/change-notes/2020-10-14-spew.md @@ -1,2 +1,2 @@ lgtm,codescanning -* Added support for the Spew deep pretty printing framework +* Added support for the Spew deep pretty-printing framework. This may cause the `go/clear-text-logging` query to return more results when sensitive data is exposed using this library.