From 348f8c16d1336f0f89eb85d513bac7037259cb47 Mon Sep 17 00:00:00 2001 From: Aditya Sharad Date: Tue, 2 Mar 2021 18:04:46 -0800 Subject: [PATCH 1/4] Actions: Add workflow to request docs review When a PR is labelled with 'ready-for-docs-review', this workflow comments on the PR to notify the GitHub CodeQL docs team. Runs on `pull_request_target` events so it can write comments to the PR. Since this runs in the context of the base repo, it must not check out the PR or use untrusted data from the event payload. --- .github/workflows/docs-review.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/docs-review.yml diff --git a/.github/workflows/docs-review.yml b/.github/workflows/docs-review.yml new file mode 100644 index 00000000000..1855924b63e --- /dev/null +++ b/.github/workflows/docs-review.yml @@ -0,0 +1,29 @@ +# When a PR is labelled with 'ready-for-docs-review', +# this workflow comments on the PR to notify the GitHub CodeQL docs team. +name: Request docs review +on: + # Runs in the context of the base repo. + # This gives the workflow write access to comment on PRs. + # The workflow should not check out or build the given ref, + # or use untrusted data from the event payload in a command line. + pull_request_target: + types: [labeled] + +jobs: + request-docs-review: + name: Request docs review + # Run only on labelled PRs to the main repository. + # Do not run on PRs to forks. + if: + github.event.label.name == 'ready-for-docs-review' + && github.event.pull_request.draft == false + && github.event.pull_request.base.repo.full_name == 'github/codeql-go' + runs-on: ubuntu-latest + steps: + - name: Comment to request docs review + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + gh pr comment "$PR_NUMBER" --repo "github/codeql-go" \ + --body "Hello @github/docs-content-codeql: this PR is ready for docs review." From 0a48fef0e701dbadbf50ef408a7976837b107aa5 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 5 Mar 2021 15:55:44 +0000 Subject: [PATCH 2/4] Model Apply methods correctly They were accidentally modeled as functions --- ql/src/semmle/go/frameworks/EvanphxJsonPatch.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/src/semmle/go/frameworks/EvanphxJsonPatch.qll b/ql/src/semmle/go/frameworks/EvanphxJsonPatch.qll index b97561fd80e..7904fea7a03 100644 --- a/ql/src/semmle/go/frameworks/EvanphxJsonPatch.qll +++ b/ql/src/semmle/go/frameworks/EvanphxJsonPatch.qll @@ -45,12 +45,12 @@ private module EvanphxJsonPatch { } } - private class Apply extends TaintTracking::FunctionModel { + private class Apply extends TaintTracking::FunctionModel, Method { Apply() { exists(string fn | fn in ["Apply", "ApplyWithOptions", "ApplyIndent", "ApplyIndentWithOptions"] | - this.hasQualifiedName(packagePath(), fn) + this.hasQualifiedName(packagePath(), "Patch", fn) ) } From 86052520a5f7ba4631ac0b1e6a0185f81d2d3c56 Mon Sep 17 00:00:00 2001 From: Aditya Sharad <6874315+adityasharad@users.noreply.github.com> Date: Mon, 8 Mar 2021 09:18:59 -0800 Subject: [PATCH 3/4] Actions: Fix comment that tags the Docs team --- .github/workflows/docs-review.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs-review.yml b/.github/workflows/docs-review.yml index 1855924b63e..8f93e75ebdb 100644 --- a/.github/workflows/docs-review.yml +++ b/.github/workflows/docs-review.yml @@ -26,4 +26,4 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} run: | gh pr comment "$PR_NUMBER" --repo "github/codeql-go" \ - --body "Hello @github/docs-content-codeql: this PR is ready for docs review." + --body "Hello @github/docs-content-codeql - this PR is ready for docs review." From 5b09d35668cbbfc9325bc12429e5dc4854677d92 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 11 Mar 2021 11:53:02 +0000 Subject: [PATCH 4/4] Add missing QLDoc for public declarations --- .../WeakCryptoAlgorithmCustomizations.qll | 5 +++++ ql/src/semmle/go/Expr.qll | 1 + ql/src/semmle/go/frameworks/Beego.qll | 4 ++++ ql/src/semmle/go/frameworks/BeegoOrm.qll | 4 ++++ ql/src/semmle/go/frameworks/GoRestfulHttp.qll | 4 ++++ ql/src/semmle/go/frameworks/SQL.qll | 3 +++ .../go/frameworks/SystemCommandExecutors.qll | 4 ++++ ql/src/semmle/go/frameworks/WebSocket.qll | 15 +++++++++++++++ ql/src/semmle/go/frameworks/XPath.qll | 3 +++ ql/src/semmle/go/security/CommandInjection.qll | 7 ++++++- .../InsecureRandomnessCustomizations.qll | 16 ++++++++++++++-- 11 files changed, 63 insertions(+), 3 deletions(-) diff --git a/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll b/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll index 81bd15a92f3..7348d8afb8c 100644 --- a/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll +++ b/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qll @@ -8,6 +8,11 @@ import go private import semmle.go.security.SensitiveActions private import CryptoLibraries +/** + * Provides default sources, sinks and sanitizers for reasoning about + * sensitive information in weak cryptographic algorithms, + * as well as extension points for adding your own. + */ module WeakCryptoAlgorithm { /** * A data flow source for sensitive information in weak cryptographic algorithms. diff --git a/ql/src/semmle/go/Expr.qll b/ql/src/semmle/go/Expr.qll index a2bc9b21cba..3d2cf956c11 100644 --- a/ql/src/semmle/go/Expr.qll +++ b/ql/src/semmle/go/Expr.qll @@ -344,6 +344,7 @@ class RuneLit = CharLit; class StringLit extends @stringlit, BasicLit { override string getAPrimaryQlClass() { result = "StringLit" } + /** Holds if this string literal is a raw string literal. */ predicate isRaw() { this.getText().matches("`%`") } } diff --git a/ql/src/semmle/go/frameworks/Beego.qll b/ql/src/semmle/go/frameworks/Beego.qll index a893da684ed..74fbdc842dd 100644 --- a/ql/src/semmle/go/frameworks/Beego.qll +++ b/ql/src/semmle/go/frameworks/Beego.qll @@ -7,6 +7,10 @@ import go import semmle.go.security.Xss private import semmle.go.security.SafeUrlFlowCustomizations +/** + * Provides classes for working with untrusted flow sources, sinks and taint propagators + * from the [Beego](`github.com/beego/beego`) package. + */ module Beego { /** Gets the module path `github.com/astaxie/beego` or `github.com/beego/beego`. */ bindingset[result] diff --git a/ql/src/semmle/go/frameworks/BeegoOrm.qll b/ql/src/semmle/go/frameworks/BeegoOrm.qll index c6410bd117d..61eb17cc13f 100644 --- a/ql/src/semmle/go/frameworks/BeegoOrm.qll +++ b/ql/src/semmle/go/frameworks/BeegoOrm.qll @@ -6,6 +6,10 @@ import go private import semmle.go.security.StoredXssCustomizations +/** + * Provides classes for working with untrusted flow sources, sinks and taint propagators + * from the [Beego ORM](`github.com/astaxie/beego/orm`) subpackage. + */ module BeegoOrm { /** Gets the package name `github.com/astaxie/beego/orm`. */ bindingset[result] diff --git a/ql/src/semmle/go/frameworks/GoRestfulHttp.qll b/ql/src/semmle/go/frameworks/GoRestfulHttp.qll index 05ad3095376..2b102d534cb 100644 --- a/ql/src/semmle/go/frameworks/GoRestfulHttp.qll +++ b/ql/src/semmle/go/frameworks/GoRestfulHttp.qll @@ -1,3 +1,7 @@ +/** + * Provides models of the [go-restful library](https://github.com/emicklei/go-restful). + */ + import go /** diff --git a/ql/src/semmle/go/frameworks/SQL.qll b/ql/src/semmle/go/frameworks/SQL.qll index 191f833d45d..b2558ead930 100644 --- a/ql/src/semmle/go/frameworks/SQL.qll +++ b/ql/src/semmle/go/frameworks/SQL.qll @@ -217,6 +217,9 @@ module SQL { } } +/** + * Provides classes for working with the [GORM](https://gorm.io/) package. + */ module Gorm { /** Gets the package name for Gorm. */ bindingset[result] diff --git a/ql/src/semmle/go/frameworks/SystemCommandExecutors.qll b/ql/src/semmle/go/frameworks/SystemCommandExecutors.qll index af121ae151d..59038ba3657 100644 --- a/ql/src/semmle/go/frameworks/SystemCommandExecutors.qll +++ b/ql/src/semmle/go/frameworks/SystemCommandExecutors.qll @@ -77,6 +77,10 @@ private class GoShCommandExecution extends SystemCommandExecution::Range, DataFl override DataFlow::Node getCommandName() { result = this.getArgument(0) } } +/** + * Provides classes for working with the + * [golang.org/x/crypto/ssh](https://pkg.go.dev/golang.org/x/crypto/ssh) package. + */ module CryptoSsh { /** Gets the package path `golang.org/x/crypto/ssh`. */ bindingset[result] diff --git a/ql/src/semmle/go/frameworks/WebSocket.qll b/ql/src/semmle/go/frameworks/WebSocket.qll index 02ab3940210..060d83e5fd5 100644 --- a/ql/src/semmle/go/frameworks/WebSocket.qll +++ b/ql/src/semmle/go/frameworks/WebSocket.qll @@ -298,24 +298,39 @@ module WebSocketReader { } } +/** + * Provides classes for working with the [Gorilla WebSocket](https://github.com/gorilla/websocket) + * package. + */ module GorillaWebsocket { /** Gets the package name `github.com/gorilla/websocket`. */ bindingset[result] string packagePath() { result = package("github.com/gorilla", "websocket") } } +/** + * Provides classes for working with the + * [golang.org/x/net/websocket](https://pkg.go.dev/golang.org/x/net/websocket) package. + */ module GolangOrgXNetWebsocket { /** Gets the package name `golang.org/x/net/websocket`. */ bindingset[result] string packagePath() { result = package("golang.org/x/net", "websocket") } } +/** + * Provides classes for working with the [nhooyr.io/websocket](http://nhooyr.io/websocket) + * package. + */ module NhooyrWebSocket { /** Gets the package name `nhooyr.io/websocket/`. */ bindingset[result] string packagePath() { result = package("nhooyr.io/websocket", "") } } +/** + * Provides classes for working with the [ws](https://github.com/gobwas/ws) package. + */ module GobwasWs { /** Gets the package name `github.com/gobwas/ws`. */ bindingset[result] diff --git a/ql/src/semmle/go/frameworks/XPath.qll b/ql/src/semmle/go/frameworks/XPath.qll index 70651c9ae75..a216feb7025 100644 --- a/ql/src/semmle/go/frameworks/XPath.qll +++ b/ql/src/semmle/go/frameworks/XPath.qll @@ -189,6 +189,9 @@ module XPath { } } +/** + * Provides classes for working with the [xmlpath](https://gopkg.in/xmlpath.v2) package. + */ module XmlPath { /** Gets the package name `github.com/go-xmlpath/xmlpath` or `gopkg.in/xmlpath`. */ bindingset[result] diff --git a/ql/src/semmle/go/security/CommandInjection.qll b/ql/src/semmle/go/security/CommandInjection.qll index 13963a2c546..6f36760b3b4 100644 --- a/ql/src/semmle/go/security/CommandInjection.qll +++ b/ql/src/semmle/go/security/CommandInjection.qll @@ -17,7 +17,8 @@ module CommandInjection { import CommandInjectionCustomizations::CommandInjection /** - * A taint-tracking configuration for reasoning about command-injection vulnerabilities. + * A taint-tracking configuration for reasoning about command-injection vulnerabilities + * with sinks which are not sanitized by `--`. */ class Configuration extends TaintTracking::Configuration { Configuration() { this = "CommandInjection" } @@ -77,6 +78,10 @@ module CommandInjection { } } + /** + * A taint-tracking configuration for reasoning about command-injection vulnerabilities + * with sinks which are sanitized by `--`. + */ class DoubleDashSanitizingConfiguration extends TaintTracking::Configuration { DoubleDashSanitizingConfiguration() { this = "CommandInjectionWithDoubleDashSanitizer" } diff --git a/ql/src/semmle/go/security/InsecureRandomnessCustomizations.qll b/ql/src/semmle/go/security/InsecureRandomnessCustomizations.qll index 57abb3aa185..db8ff731fbc 100644 --- a/ql/src/semmle/go/security/InsecureRandomnessCustomizations.qll +++ b/ql/src/semmle/go/security/InsecureRandomnessCustomizations.qll @@ -5,6 +5,10 @@ import go +/** + * Provides default sources, sinks and sanitizers for reasoning about random values that are + * not cryptographically secure, as well as extension points for adding your own. + */ module InsecureRandomness { /** * A data flow source for insufficient random sources @@ -32,6 +36,10 @@ module InsecureRandomness { InsecureRandomSource() { this.getTarget().getPackage().getPath() = "math/rand" } } + /** + * Gets an interface outside of the `crypto` package which is the same as an + * interface in the `crypto` package. + */ string nonCryptoInterface() { result = ["io.Writer", "io.Reader", "sync.Mutex", "net.Listener"] } /** @@ -47,8 +55,11 @@ module InsecureRandomness { pkg.regexpMatch("crypto/.*") and not pkg = getAHashPkg() and not (pkg = "crypto/rand" and name = "Read") and - not (pkg = "crypto/cipher" and name = ["Read", "Write"]) and // crypto/cipher APIs for reading/writing encrypted streams - not fn.hasQualifiedName(nonCryptoInterface(), _) and // some interfaces in crypto are the same as interfaces elsewhere, e.g. tls.listener is the same as net.Listener + // `crypto/cipher` APIs for reading/writing encrypted streams + not (pkg = "crypto/cipher" and name = ["Read", "Write"]) and + // Some interfaces in the `crypto` package are the same as interfaces + // elsewhere, e.g. tls.listener is the same as net.Listener + not fn.hasQualifiedName(nonCryptoInterface(), _) and this = fn.getACall().getAnArgument() ) } @@ -71,6 +82,7 @@ module InsecureRandomness { override string getKind() { result = "a password-related function" } } + /** Gets a package that implements hash algorithms. */ bindingset[result] private string getAHashPkg() { result.regexpMatch("crypto/(md5|sha(1|256|512)|rand)") }