From e034458574aa26d4d548f54d2a92eaeaa14e2937 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 13 May 2020 15:25:54 +0100 Subject: [PATCH 1/5] Fix MongoDB tests. --- ql/test/library-tests/semmle/go/frameworks/NoSQL/main.go | 2 +- .../CWE-089/vendor/go.mongodb.org/mongo-driver/mongo/stub.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/test/library-tests/semmle/go/frameworks/NoSQL/main.go b/ql/test/library-tests/semmle/go/frameworks/NoSQL/main.go index 438763a587e..a437eef781b 100644 --- a/ql/test/library-tests/semmle/go/frameworks/NoSQL/main.go +++ b/ql/test/library-tests/semmle/go/frameworks/NoSQL/main.go @@ -10,7 +10,7 @@ import ( "go.mongodb.org/mongo-driver/mongo" ) -func test(coll *mongo.Collection, filter interface{}, models []WriteModel, ctx context.Context) { +func test(coll *mongo.Collection, filter interface{}, models []mongo.WriteModel, ctx context.Context) { fieldName := "test" document := filter diff --git a/ql/test/query-tests/Security/CWE-089/vendor/go.mongodb.org/mongo-driver/mongo/stub.go b/ql/test/query-tests/Security/CWE-089/vendor/go.mongodb.org/mongo-driver/mongo/stub.go index 1a06732dbb1..013a9e90963 100644 --- a/ql/test/query-tests/Security/CWE-089/vendor/go.mongodb.org/mongo-driver/mongo/stub.go +++ b/ql/test/query-tests/Security/CWE-089/vendor/go.mongodb.org/mongo-driver/mongo/stub.go @@ -193,7 +193,7 @@ func (_ *Collection) Watch(_ context.Context, _ interface{}, _ ...*interface{}) return nil, nil } -func Connect(_ context.Context, _ ...*interface{}) (*Client, error) { +func Connect(_ context.Context, _ ...interface{}) (*Client, error) { return nil, nil } From ac9e39120b362f60b2580d9fcd3825b64b1934ad Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 13 May 2020 15:26:16 +0100 Subject: [PATCH 2/5] Fix unused variable in test. --- ql/test/query-tests/Security/CWE-022/tst.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ql/test/query-tests/Security/CWE-022/tst.go b/ql/test/query-tests/Security/CWE-022/tst.go index 8cb4fe5ee56..599faccf0f1 100644 --- a/ql/test/query-tests/Security/CWE-022/tst.go +++ b/ql/test/query-tests/Security/CWE-022/tst.go @@ -15,7 +15,7 @@ func uploadFile(w http.ResponseWriter, r *http.Request) { // err handling defer file.Close() tempFile, _ := ioutil.TempFile("/tmp", handler.Filename) // NOT OK - // do stuff with tempFile + use(tempFile) } func unzip2(f string, root string) { @@ -50,3 +50,5 @@ func containedIn(f string, root string) bool { } return false } + +func use(v interface{}) {} From ec2314310e5d2a304bfed3cbf17cf3060662e4b7 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 13 May 2020 15:29:30 +0100 Subject: [PATCH 3/5] Fix code example in query. --- .../experimental/CWE-807/SensitiveConditionBypassBad.go | 6 +++++- ql/src/semmle/go/frameworks/NoSQL.qll | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ql/src/experimental/CWE-807/SensitiveConditionBypassBad.go b/ql/src/experimental/CWE-807/SensitiveConditionBypassBad.go index e2ca02615db..bf8e70f88b7 100644 --- a/ql/src/experimental/CWE-807/SensitiveConditionBypassBad.go +++ b/ql/src/experimental/CWE-807/SensitiveConditionBypassBad.go @@ -1,4 +1,8 @@ -func ex3(w http.ResponseWriter, r *http.Request) { +package main + +import "net/http" + +func example(w http.ResponseWriter, r *http.Request) { test2 := "test" if r.Header.Get("X-Password") != test2 { login() diff --git a/ql/src/semmle/go/frameworks/NoSQL.qll b/ql/src/semmle/go/frameworks/NoSQL.qll index a49c3864b62..10444e67451 100644 --- a/ql/src/semmle/go/frameworks/NoSQL.qll +++ b/ql/src/semmle/go/frameworks/NoSQL.qll @@ -96,9 +96,12 @@ module NoSQL { } } - predicate isAdditionalMongoTaintStep(DataFlow::Node prev, DataFlow::Node succ) { - // Taint bson.E if input is tainted - exists(Write w, DataFlow::Node base, Field f | w.writesField(base, f, prev) | + /** + * Holds if taint flows from `pred` to `succ` through a MongoDB-specific API. + */ + predicate isAdditionalMongoTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + // Taint an entry if the `Value` is tainted + exists(Write w, DataFlow::Node base, Field f | w.writesField(base, f, pred) | base = succ.getASuccessor*() and base.getType().hasQualifiedName(mongoBsonPrimitive(), "E") and f.getName() = "Value" From 6e58524b7885b08f7616b3b013c82a0ec0d44261 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 13 May 2020 15:40:31 +0100 Subject: [PATCH 4/5] Fix a typo. --- ql/src/semmle/go/frameworks/NoSQL.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/src/semmle/go/frameworks/NoSQL.qll b/ql/src/semmle/go/frameworks/NoSQL.qll index 10444e67451..46c73d6ad40 100644 --- a/ql/src/semmle/go/frameworks/NoSQL.qll +++ b/ql/src/semmle/go/frameworks/NoSQL.qll @@ -9,7 +9,7 @@ module NoSQL { /** * A data-flow node whose string value is interpreted as (part of) a NoSQL query. * - * Extends this class to refine existing API models. If you want to model new APIs, + * Extend this class to refine existing API models. If you want to model new APIs, * extend `NoSQL::QueryString::Range` instead. */ class NoSQLQueryString extends DataFlow::Node { From 41b5fc17abd494b374a79119a76674f402e3757d Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 13 May 2020 15:40:36 +0100 Subject: [PATCH 5/5] Inline two single-use predicates. This fixes a TODO. --- ql/src/semmle/go/frameworks/NoSQL.qll | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ql/src/semmle/go/frameworks/NoSQL.qll b/ql/src/semmle/go/frameworks/NoSQL.qll index 46c73d6ad40..d80fe3f20b7 100644 --- a/ql/src/semmle/go/frameworks/NoSQL.qll +++ b/ql/src/semmle/go/frameworks/NoSQL.qll @@ -18,11 +18,6 @@ module NoSQL { NoSQLQueryString() { this = self } } - //TODO : Replace the following two predicate definitions with a simple call to package() - private string mongoDb() { result = "go.mongodb.org/mongo-driver/mongo" } - - private string mongoBsonPrimitive() { result = "go.mongodb.org/mongo-driver/bson/primitive" } - /** Provides classes for working with SQL query strings. */ module NoSQLQueryString { /** @@ -89,7 +84,7 @@ module NoSQL { MongoDbCollectionQueryString() { exists(Method meth, string methodName, int n | collectionMethods(methodName, n) and - meth.hasQualifiedName(mongoDb(), "Collection", methodName) and + meth.hasQualifiedName("go.mongodb.org/mongo-driver/mongo", "Collection", methodName) and this = meth.getACall().getArgument(n) ) } @@ -103,7 +98,7 @@ module NoSQL { // Taint an entry if the `Value` is tainted exists(Write w, DataFlow::Node base, Field f | w.writesField(base, f, pred) | base = succ.getASuccessor*() and - base.getType().hasQualifiedName(mongoBsonPrimitive(), "E") and + base.getType().hasQualifiedName("go.mongodb.org/mongo-driver/bson/primitive", "E") and f.getName() = "Value" ) }