From 01ad7acb6f40b3830fe66e0d6e1e25f428b15fd5 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Mon, 14 Sep 2020 16:48:44 +0100 Subject: [PATCH 01/19] Remove unnecessary import --- ql/test/library-tests/semmle/go/frameworks/Gin/Gin.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.ql b/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.ql index 67a3e87b360..100c3a8edc3 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.ql +++ b/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.ql @@ -1,4 +1,3 @@ import go -import semmle.go.frameworks.Gin select any(UntrustedFlowSource src) From f4f29be8ac57178c5c6f5b01010e3491e6b47033 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Sep 2020 13:41:08 +0100 Subject: [PATCH 02/19] Add ability to specify default taint sanitizers This allows library models to specify taint sanitizers. --- ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll index f16b0cc4af5..0defbe66e1f 100644 --- a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -167,8 +167,10 @@ predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) { localAdditionalTaintStep(src, sink) } +abstract class DefaultTaintSanitizer extends DataFlow::Node { } + /** * Holds if `node` should be a sanitizer in all global taint flow configurations * but not in local taint. */ -predicate defaultTaintSanitizer(DataFlow::Node node) { none() } +predicate defaultTaintSanitizer(DataFlow::Node node) { node instanceof DefaultTaintSanitizer } From 4dfa9d58c031d75885dc11ea87a8dd330f39c456 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Sep 2020 13:41:20 +0100 Subject: [PATCH 03/19] Model Revel --- ql/src/go.qll | 1 + ql/src/semmle/go/frameworks/Revel.qll | 97 +++ ql/src/semmle/go/frameworks/WebSocket.qll | 24 + .../semmle/go/frameworks/Revel/Revel.go | 111 +++ .../go/frameworks/Revel/TaintFlows.expected | 34 + .../semmle/go/frameworks/Revel/TaintFlows.ql | 19 + .../semmle/go/frameworks/Revel/go.mod | 20 + .../vendor/github.com/revel/revel/stub.go | 705 ++++++++++++++++++ .../go/frameworks/Revel/vendor/modules.txt | 42 ++ 9 files changed, 1053 insertions(+) create mode 100644 ql/src/semmle/go/frameworks/Revel.qll create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.ql create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/go.mod create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt diff --git a/ql/src/go.qll b/ql/src/go.qll index b4b07eff90f..6f400db0537 100644 --- a/ql/src/go.qll +++ b/ql/src/go.qll @@ -39,6 +39,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.Revel import semmle.go.frameworks.Spew import semmle.go.frameworks.SQL import semmle.go.frameworks.Stdlib diff --git a/ql/src/semmle/go/frameworks/Revel.qll b/ql/src/semmle/go/frameworks/Revel.qll new file mode 100644 index 00000000000..365c30fc2ed --- /dev/null +++ b/ql/src/semmle/go/frameworks/Revel.qll @@ -0,0 +1,97 @@ +/** + * Provides classes for working with untrusted flow sources from the `github.com/revel/revel` package. + */ + +import go + +module Revel { + /** Gets the package name. */ + bindingset[result] + string packagePath() { result = package(["github.com/revel", "github.com/robfig"], "revel") } + + private class ControllerParams extends UntrustedFlowSource::Range, DataFlow::FieldReadNode { + ControllerParams() { + exists(Field f | + this.readsField(_, f) and + f.hasQualifiedName(packagePath(), "Controller", "Params") + ) + } + } + + private class ParamsFixedSanitizer extends TaintTracking::DefaultTaintSanitizer, + DataFlow::FieldReadNode { + ParamsFixedSanitizer() { + exists(Field f | + this.readsField(_, f) and + f.hasQualifiedName(packagePath(), "Params", "Fixed") + ) + } + } + + private class ParamsBind extends TaintTracking::FunctionModel, Method { + ParamsBind() { this.hasQualifiedName(packagePath(), "Params", ["Bind", "BindJSON"]) } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isReceiver() and outp.isParameter(0) + } + } + + private class RouteMatchParams extends UntrustedFlowSource::Range, DataFlow::FieldReadNode { + RouteMatchParams() { + exists(Field f | + this.readsField(_, f) and + f.hasQualifiedName(packagePath(), "RouteMatch", "Params") + ) + } + } + + /** An access to an HTTP request field whose value may be controlled by an untrusted user. */ + private class UserControlledRequestField extends UntrustedFlowSource::Range, + DataFlow::FieldReadNode { + UserControlledRequestField() { + exists(string fieldName | + this.getField().hasQualifiedName(packagePath(), "Request", fieldName) + | + fieldName in ["In", "Header", "URL", "Form", "MultipartForm"] + ) + } + } + + private class UserControlledRequestMethod extends UntrustedFlowSource::Range, + DataFlow::MethodCallNode { + UserControlledRequestMethod() { + this + .getTarget() + .hasQualifiedName(packagePath(), "Request", + ["FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody"]) + } + } + + private class ServerMultipartFormGetFiles extends TaintTracking::FunctionModel, Method { + ServerMultipartFormGetFiles() { + this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetFiles") + } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isReceiver() and outp.isResult() + } + } + + private class ServerMultipartFormGetValues extends TaintTracking::FunctionModel, Method { + ServerMultipartFormGetValues() { + this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetValues") + } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isReceiver() and outp.isResult() + } + } + + private class ServerRequestGet extends TaintTracking::FunctionModel, Method { + ServerRequestGet() { this.hasQualifiedName(packagePath(), "ServerRequest", "Get") } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isReceiver() and outp.isResult(0) + } + } +} diff --git a/ql/src/semmle/go/frameworks/WebSocket.qll b/ql/src/semmle/go/frameworks/WebSocket.qll index 0919ae2aafa..fc2278d785c 100644 --- a/ql/src/semmle/go/frameworks/WebSocket.qll +++ b/ql/src/semmle/go/frameworks/WebSocket.qll @@ -271,4 +271,28 @@ module WebSocketReader { override FunctionOutput getAnOutput() { result.isResult(1) } } + + /** + * The `ServerWebSocket.MessageReceive` method of the `github.com/revel/revel` package. + */ + private class RevelServerWebSocketMessageReceive extends Range, Method { + RevelServerWebSocketMessageReceive() { + // func MessageReceive(v interface{}) error + this.hasQualifiedName(Revel::packagePath(), "ServerWebSocket", "MessageReceive") + } + + override FunctionOutput getAnOutput() { result.isParameter(0) } + } + + /** + * The `ServerWebSocket.MessageReceiveJSON` method of the `github.com/revel/revel` package. + */ + private class RevelServerWebSocketMessageReceiveJSON extends Range, Method { + RevelServerWebSocketMessageReceiveJSON() { + // func MessageReceiveJSON(v interface{}) error + this.hasQualifiedName(Revel::packagePath(), "ServerWebSocket", "MessageReceiveJSON") + } + + override FunctionOutput getAnOutput() { result.isParameter(0) } + } } diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go new file mode 100644 index 00000000000..b067d0238e5 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go @@ -0,0 +1,111 @@ +package main + +//go:generate depstubber -vendor github.com/revel/revel Controller,Params,Request,Router HTTP_QUERY + +import ( + "io/ioutil" + "mime/multipart" + "net/url" + + "github.com/revel/revel" +) + +func main() {} + +type myAppController struct { + *revel.Controller + OtherStuff string +} + +type person struct { + Name string `form:"name"` + Address string `form:"address"` +} + +func useString(val string) {} + +func useFiles(val *multipart.FileHeader) {} + +func useJSON(val []byte) {} + +func useURLValues(v url.Values) { + useString(v["key"][0]) + useString(v.Get("key")) +} + +func usePerson(p person) {} + +func (c myAppController) accessingParamsDirectlyIsUnsafe() { + useString(c.Params.Get("key")) // NOT OK + useURLValues(c.Params.Values) // NOT OK + + val4 := "" + c.Params.Bind(&val4, "key") // NOT OK + useString(val4) + + useString(c.Request.FormValue("key")) // NOT OK +} + +func (c myAppController) accessingFixedIsSafe(mainRouter *revel.Router) { + useURLValues(c.Params.Fixed) // OK + useString(mainRouter.Route(c.Request).FixedParams[0]) // OK +} + +func (c myAppController) accessingRouteIsUnsafe(mainRouter *revel.Router) { + useURLValues(c.Params.Route) // NOT OK + useURLValues(mainRouter.Route(c.Request).Params) // NOT OK +} + +func (c myAppController) accessingParamsQueryIsUnsafe() { + useURLValues(c.Params.Query) // NOT OK +} + +func (c myAppController) accessingParamsFormIsUnsafe() { + useURLValues(c.Params.Form) // NOT OK + useString(c.Request.PostFormValue("key")) // NOT OK +} + +func (c myAppController) accessingParamsFilesIsUnsafe() { + useFiles(c.Params.Files["key"][0]) // NOT OK +} + +func (c myAppController) accessingParamsJSONIsUnsafe() { + useJSON(c.Params.JSON) // NOT OK + + var val2 map[string]interface{} + c.Params.BindJSON(&val2) // NOT OK + useString(val2["name"].(string)) +} + +func accessingRequestDirectlyIsUnsafe(c *revel.Controller) { + useURLValues(c.Request.GetQuery()) // NOT OK + useURLValues(c.Request.Form) // NOT OK + useURLValues(c.Request.MultipartForm.Value) // NOT OK + + form, _ := c.Request.GetForm() // NOT OK + useURLValues(form) + + smp1, _ := c.Request.GetMultipartForm() // NOT OK + useURLValues(smp1.GetValues()) + + smp2, _ := c.Request.GetMultipartForm() // NOT OK + useFiles(smp2.GetFiles()["key"][0]) + + useFiles(c.Request.MultipartForm.File["key"][0]) // NOT OK + + json, _ := ioutil.ReadAll(c.Request.GetBody()) // NOT OK + useJSON(json) +} + +func accessingServerRequest(c *revel.Controller) { + query, _ := c.Request.In.Get(revel.HTTP_QUERY) // NOT OK + useURLValues(query.(url.Values)) + + var message string + c.Request.WebSocket.MessageReceive(&message) // NOT OK + useString(message) + + var p person + c.Request.WebSocket.MessageReceiveJSON(&p) // NOT OK + usePerson(p) +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected new file mode 100644 index 00000000000..1b58fbfb755 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected @@ -0,0 +1,34 @@ +| Revel.go:39:12:39:19 | selection of Params : pointer type | Revel.go:39:12:39:30 | call to Get | 39 | +| Revel.go:40:15:40:22 | selection of Params : pointer type | Revel.go:32:12:32:22 | index expression | 40 | +| Revel.go:40:15:40:22 | selection of Params : pointer type | Revel.go:33:12:33:23 | call to Get | 40 | +| Revel.go:43:2:43:9 | selection of Params : pointer type | Revel.go:44:12:44:15 | val4 | 43 | +| Revel.go:46:12:46:37 | call to FormValue | Revel.go:46:12:46:37 | call to FormValue | 46 | +| Revel.go:55:15:55:22 | selection of Params : pointer type | Revel.go:32:12:32:22 | index expression | 55 | +| Revel.go:55:15:55:22 | selection of Params : pointer type | Revel.go:33:12:33:23 | call to Get | 55 | +| Revel.go:56:15:56:48 | selection of Params : map type | Revel.go:32:12:32:22 | index expression | 56 | +| Revel.go:56:15:56:48 | selection of Params : map type | Revel.go:33:12:33:23 | call to Get | 56 | +| Revel.go:60:15:60:22 | selection of Params : pointer type | Revel.go:32:12:32:22 | index expression | 60 | +| Revel.go:60:15:60:22 | selection of Params : pointer type | Revel.go:33:12:33:23 | call to Get | 60 | +| Revel.go:64:15:64:22 | selection of Params : pointer type | Revel.go:32:12:32:22 | index expression | 64 | +| Revel.go:64:15:64:22 | selection of Params : pointer type | Revel.go:33:12:33:23 | call to Get | 64 | +| Revel.go:65:12:65:41 | call to PostFormValue | Revel.go:65:12:65:41 | call to PostFormValue | 65 | +| Revel.go:69:11:69:18 | selection of Params : pointer type | Revel.go:69:11:69:34 | index expression | 69 | +| Revel.go:73:10:73:17 | selection of Params : pointer type | Revel.go:73:10:73:22 | selection of JSON | 73 | +| Revel.go:76:2:76:9 | selection of Params : pointer type | Revel.go:77:12:77:32 | type assertion | 76 | +| Revel.go:81:15:81:34 | call to GetQuery : Values | Revel.go:32:12:32:22 | index expression | 81 | +| Revel.go:81:15:81:34 | call to GetQuery : Values | Revel.go:33:12:33:23 | call to Get | 81 | +| Revel.go:82:15:82:28 | selection of Form : Values | Revel.go:32:12:32:22 | index expression | 82 | +| Revel.go:82:15:82:28 | selection of Form : Values | Revel.go:33:12:33:23 | call to Get | 82 | +| Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:32:12:32:22 | index expression | 83 | +| Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:33:12:33:23 | call to Get | 83 | +| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:32:12:32:22 | index expression | 85 | +| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:33:12:33:23 | call to Get | 85 | +| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:32:12:32:22 | index expression | 88 | +| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:33:12:33:23 | call to Get | 88 | +| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:92:11:92:35 | index expression | 91 | +| Revel.go:94:11:94:33 | selection of MultipartForm : pointer type | Revel.go:94:11:94:48 | index expression | 94 | +| Revel.go:96:28:96:46 | call to GetBody : Reader | Revel.go:97:10:97:13 | json | 96 | +| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:32:12:32:22 | index expression | 101 | +| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:33:12:33:23 | call to Get | 101 | +| Revel.go:105:37:105:44 | &... : pointer type | Revel.go:106:12:106:18 | message | 105 | +| Revel.go:109:41:109:42 | &... : pointer type | Revel.go:110:12:110:12 | p | 109 | diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.ql b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.ql new file mode 100644 index 00000000000..e41767f0a97 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.ql @@ -0,0 +1,19 @@ +import go + +class SinkFunction extends Function { + SinkFunction() { this.getName() = ["useFiles", "useJSON", "usePerson", "useString"] } +} + +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() + } +} + +from TaintTracking::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink, int i +where config.hasFlowPath(source, sink) and source.hasLocationInfo(_, i, _, _, _) +select source, sink, i order by i diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod b/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod new file mode 100644 index 00000000000..a0968095036 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod @@ -0,0 +1,20 @@ +module codeql-go-tests/frameworks/Revel + +go 1.14 + +require ( + github.com/fsnotify/fsnotify v1.4.9 // indirect + github.com/github/depstubber v0.0.0-20200910105848-4f6b1e61222c // indirect + github.com/go-stack/stack v1.8.0 // indirect + github.com/inconshreveable/log15 v0.0.0-20200109203555-b30bc20e4fd1 // indirect + github.com/mattn/go-colorable v0.1.7 // indirect + github.com/revel/config v1.0.0 // indirect + github.com/revel/log15 v2.11.20+incompatible // indirect + github.com/revel/pathtree v0.0.0-20140121041023-41257a1839e9 // indirect + github.com/revel/revel v1.0.0 + github.com/twinj/uuid v1.0.0 // indirect + github.com/xeonx/timeago v1.0.0-rc4 // indirect + golang.org/x/net v0.0.0-20200904194848-62affa334b73 // indirect + gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect + gopkg.in/stack.v0 v0.0.0-20141108040640-9b43fcefddd0 // indirect +) diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go new file mode 100644 index 00000000000..c2bfe8c15ef --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go @@ -0,0 +1,705 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/revel/revel, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/revel/revel (exports: Controller,Params,Request,Router; functions: HTTP_QUERY) + +// Package revel is a stub of github.com/revel/revel, generated by depstubber. +package revel + +import ( + context "context" + io "io" + multipart "mime/multipart" + http "net/http" + url "net/url" + os "os" + reflect "reflect" + regexp "regexp" + time "time" +) + +type AcceptLanguage struct { + Language string + Quality float32 +} + +type AcceptLanguages []AcceptLanguage + +func (_ AcceptLanguages) Len() int { + return 0 +} + +func (_ AcceptLanguages) Less(_ int, _ int) bool { + return false +} + +func (_ AcceptLanguages) String() string { + return "" +} + +func (_ AcceptLanguages) Swap(_ int, _ int) {} + +type ActionDefinition struct { + Host string + Method string + URL string + Action string + Star bool + Args map[string]string +} + +func (_ *ActionDefinition) String() string { + return "" +} + +type ContentDisposition string + +type Controller struct { + Name string + Type *ControllerType + MethodName string + MethodType *MethodType + AppController interface{} + Action string + ClientIP string + Request *Request + Response *Response + Result Result + Flash Flash + Session interface{} + Params *Params + Args map[string]interface{} + ViewArgs map[string]interface{} + Validation *Validation + Log interface{} +} + +func (_ *Controller) Destroy() {} + +func (_ *Controller) FlashParams() {} + +func (_ *Controller) Forbidden(_ string, _ ...interface{}) Result { + return nil +} + +func (_ *Controller) Message(_ string, _ ...interface{}) string { + return "" +} + +func (_ *Controller) NotFound(_ string, _ ...interface{}) Result { + return nil +} + +func (_ *Controller) Redirect(_ interface{}, _ ...interface{}) Result { + return nil +} + +func (_ *Controller) Render(_ ...interface{}) Result { + return nil +} + +func (_ *Controller) RenderBinary(_ io.Reader, _ string, _ ContentDisposition, _ time.Time) Result { + return nil +} + +func (_ *Controller) RenderError(_ error) Result { + return nil +} + +func (_ *Controller) RenderFile(_ *os.File, _ ContentDisposition) Result { + return nil +} + +func (_ *Controller) RenderFileName(_ string, _ ContentDisposition) Result { + return nil +} + +func (_ *Controller) RenderHTML(_ string) Result { + return nil +} + +func (_ *Controller) RenderJSON(_ interface{}) Result { + return nil +} + +func (_ *Controller) RenderJSONP(_ string, _ interface{}) Result { + return nil +} + +func (_ *Controller) RenderTemplate(_ string) Result { + return nil +} + +func (_ *Controller) RenderText(_ string, _ ...interface{}) Result { + return nil +} + +func (_ *Controller) RenderXML(_ interface{}) Result { + return nil +} + +func (_ *Controller) SetAction(_ string, _ string) error { + return nil +} + +func (_ *Controller) SetController(_ ServerContext) {} + +func (_ *Controller) SetCookie(_ *http.Cookie) {} + +func (_ *Controller) SetTypeAction(_ string, _ string, _ *ControllerType) error { + return nil +} + +func (_ *Controller) Stats() map[string]interface{} { + return nil +} + +func (_ *Controller) TemplateOutput(_ string) ([]byte, error) { + return nil, nil +} + +func (_ *Controller) Todo() Result { + return nil +} + +type ControllerFieldPath struct { + IsPointer bool + FieldIndexPath []int + FunctionCall reflect.Value +} + +func (_ *ControllerFieldPath) Invoke(_ reflect.Value, _ []reflect.Value) []reflect.Value { + return nil +} + +type ControllerType struct { + Namespace string + ModuleSource *Module + Type reflect.Type + Methods []*MethodType + ControllerIndexes [][]int + ControllerEvents *ControllerTypeEvents +} + +func (_ *ControllerType) Method(_ string) *MethodType { + return nil +} + +func (_ *ControllerType) Name() string { + return "" +} + +func (_ *ControllerType) ShortName() string { + return "" +} + +type ControllerTypeEvents struct { + Before []*ControllerFieldPath + After []*ControllerFieldPath + Finally []*ControllerFieldPath + Panic []*ControllerFieldPath +} + +type Error struct { + SourceType string + Title string + Path string + Description string + Line int + Column int + SourceLines []string + Stack string + MetaError string + Link string +} + +func (_ *Error) ContextSource() []SourceLine { + return nil +} + +func (_ *Error) Error() string { + return "" +} + +func (_ *Error) SetLink(_ string) {} + +type Flash struct { + Data map[string]string + Out map[string]string +} + +func (_ Flash) Error(_ string, _ ...interface{}) {} + +func (_ Flash) Success(_ string, _ ...interface{}) {} + +var HTTP_QUERY int = 0 + +type MethodArg struct { + Name string + Type reflect.Type +} + +type MethodType struct { + Name string + Args []*MethodArg + RenderArgNames map[int][]string + Index int +} + +type Module struct { + Name string + ImportPath string + Path string + ControllerTypeList []*ControllerType + Log interface{} +} + +func (_ *Module) AddController(_ *ControllerType) {} + +func (_ *Module) ControllerByName(_ string, _ string) *ControllerType { + return nil +} + +func (_ *Module) Namespace() string { + return "" +} + +type MultipartForm struct { + File map[string][]*multipart.FileHeader + Value url.Values +} + +type OutResponse struct { + Server ServerResponse +} + +func (_ *OutResponse) Destroy() {} + +func (_ *OutResponse) Header() *RevelHeader { + return nil +} + +func (_ *OutResponse) Write(_ []byte) (int, error) { + return 0, nil +} + +type Params struct { + Values url.Values + Fixed url.Values + Route url.Values + Query url.Values + Form url.Values + Files map[string][]*multipart.FileHeader + JSON []byte +} + +func (_ Params) Add(_ string, _ string) {} + +func (_ Params) Del(_ string) {} + +func (_ Params) Encode() string { + return "" +} + +func (_ Params) Get(_ string) string { + return "" +} + +func (_ Params) Set(_ string, _ string) {} + +func (_ *Params) Bind(_ interface{}, _ string) {} + +func (_ *Params) BindJSON(_ interface{}) error { + return nil +} + +type Request struct { + In ServerRequest + Header *RevelHeader + ContentType string + Format string + AcceptLanguages AcceptLanguages + Locale string + WebSocket ServerWebSocket + Method string + RemoteAddr string + Host string + URL *url.URL + Form url.Values + MultipartForm *MultipartForm +} + +func (_ *Request) Args() map[string]interface{} { + return nil +} + +func (_ *Request) Context() context.Context { + return nil +} + +func (_ *Request) Cookie(_ string) (ServerCookie, error) { + return nil, nil +} + +func (_ *Request) Destroy() {} + +func (_ *Request) FormValue(_ string) string { + return "" +} + +func (_ *Request) GetBody() io.Reader { + return nil +} + +func (_ *Request) GetForm() (url.Values, error) { + return nil, nil +} + +func (_ *Request) GetHttpHeader(_ string) string { + return "" +} + +func (_ *Request) GetMultipartForm() (ServerMultipartForm, error) { + return nil, nil +} + +func (_ *Request) GetPath() string { + return "" +} + +func (_ *Request) GetQuery() url.Values { + return nil +} + +func (_ *Request) GetRequestURI() string { + return "" +} + +func (_ *Request) GetValue(_ int) interface{} { + return nil +} + +func (_ *Request) MultipartReader() (*multipart.Reader, error) { + return nil, nil +} + +func (_ *Request) ParseForm() error { + return nil +} + +func (_ *Request) ParseMultipartForm(_ int64) error { + return nil +} + +func (_ *Request) PostFormValue(_ string) string { + return "" +} + +func (_ *Request) Referer() string { + return "" +} + +func (_ *Request) SetRequest(_ ServerRequest) {} + +func (_ *Request) UserAgent() string { + return "" +} + +type Response struct { + Status int + ContentType string + Out OutResponse +} + +func (_ *Response) Destroy() {} + +func (_ *Response) GetStreamWriter() StreamWriter { + return nil +} + +func (_ *Response) GetWriter() io.Writer { + return nil +} + +func (_ *Response) SetResponse(_ ServerResponse) {} + +func (_ *Response) SetStatus(_ int) {} + +func (_ *Response) SetWriter(_ io.Writer) bool { + return false +} + +func (_ *Response) WriteHeader(_ int, _ string) {} + +type Result interface { + Apply(_ *Request, _ *Response) +} + +type RevelHeader struct { + Server ServerHeader +} + +func (_ *RevelHeader) Add(_ string, _ string) {} + +func (_ *RevelHeader) Destroy() {} + +func (_ *RevelHeader) Get(_ string) string { + return "" +} + +func (_ *RevelHeader) GetAll(_ string) []string { + return nil +} + +func (_ *RevelHeader) Set(_ string, _ string) {} + +func (_ *RevelHeader) SetCookie(_ string) {} + +func (_ *RevelHeader) SetStatus(_ int) {} + +type Route struct { + ModuleSource *Module + Method string + Path string + Action string + ControllerNamespace string + ControllerName string + MethodName string + FixedParams []string + TreePath string + TypeOfController *ControllerType +} + +func (_ *Route) ActionPath() string { + return "" +} + +type RouteMatch struct { + Action string + ControllerName string + MethodName string + FixedParams []string + Params map[string][]string + TypeOfController *ControllerType + ModuleSource *Module +} + +type Router struct { + Routes []*Route + Tree interface{} + Module string +} + +func (_ *Router) Refresh() *Error { + return nil +} + +func (_ *Router) Reverse(_ string, _ map[string]string) *ActionDefinition { + return nil +} + +func (_ *Router) ReverseError(_ string, _ map[string]string, _ *Request) (*ActionDefinition, error) { + return nil, nil +} + +func (_ *Router) Route(_ *Request) *RouteMatch { + return nil +} + +type ServerContext interface { + GetRequest() ServerRequest + GetResponse() ServerResponse +} + +type ServerCookie interface { + GetValue() string +} + +type ServerHeader interface { + Add(_ string, _ string) + Del(_ string) + Get(_ string) []string + GetCookie(_ string) (ServerCookie, error) + GetKeys() []string + Set(_ string, _ string) + SetCookie(_ string) + SetStatus(_ int) +} + +type ServerMultipartForm interface { + GetFiles() map[string][]*multipart.FileHeader + GetValues() url.Values + RemoveAll() error +} + +type ServerRequest interface { + Get(_ int) (interface{}, error) + GetRaw() interface{} + Set(_ int, _ interface{}) bool +} + +type ServerResponse interface { + Get(_ int) (interface{}, error) + GetRaw() interface{} + Set(_ int, _ interface{}) bool +} + +type ServerWebSocket interface { + Get(_ int) (interface{}, error) + GetRaw() interface{} + MessageReceive(_ interface{}) error + MessageReceiveJSON(_ interface{}) error + MessageSend(_ interface{}) error + MessageSendJSON(_ interface{}) error + Set(_ int, _ interface{}) bool +} + +type SourceLine struct { + Source string + Line int + IsError bool +} + +type StreamWriter interface { + WriteStream(_ string, _ int64, _ time.Time, _ io.Reader) error +} + +type Validation struct { + Errors []*ValidationError + Request *Request + Translator func(string, string, ...interface{}) string +} + +func (_ *Validation) Check(_ interface{}, _ ...Validator) *ValidationResult { + return nil +} + +func (_ *Validation) Clear() {} + +func (_ *Validation) Domain(_ string) *ValidationResult { + return nil +} + +func (_ *Validation) Email(_ string) *ValidationResult { + return nil +} + +func (_ *Validation) Error(_ string, _ ...interface{}) *ValidationResult { + return nil +} + +func (_ *Validation) ErrorKey(_ string, _ ...interface{}) *ValidationResult { + return nil +} + +func (_ *Validation) ErrorMap() map[string]*ValidationError { + return nil +} + +func (_ *Validation) FilePath(_ string, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) HasErrors() bool { + return false +} + +func (_ *Validation) IPAddr(_ string, _ ...int) *ValidationResult { + return nil +} + +func (_ *Validation) Keep() {} + +func (_ *Validation) Length(_ interface{}, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) MacAddr(_ string) *ValidationResult { + return nil +} + +func (_ *Validation) Match(_ string, _ *regexp.Regexp) *ValidationResult { + return nil +} + +func (_ *Validation) Max(_ int, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) MaxFloat(_ float64, _ float64) *ValidationResult { + return nil +} + +func (_ *Validation) MaxSize(_ interface{}, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) Min(_ int, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) MinFloat(_ float64, _ float64) *ValidationResult { + return nil +} + +func (_ *Validation) MinSize(_ interface{}, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) PureText(_ string, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) Range(_ int, _ int, _ int) *ValidationResult { + return nil +} + +func (_ *Validation) RangeFloat(_ float64, _ float64, _ float64) *ValidationResult { + return nil +} + +func (_ *Validation) Required(_ interface{}) *ValidationResult { + return nil +} + +func (_ *Validation) URL(_ string) *ValidationResult { + return nil +} + +func (_ *Validation) ValidationResult(_ bool) *ValidationResult { + return nil +} + +type ValidationError struct { + Message string + Key string +} + +func (_ *ValidationError) String() string { + return "" +} + +type ValidationResult struct { + Error *ValidationError + Ok bool + Locale string + Translator func(string, string, ...interface{}) string +} + +func (_ *ValidationResult) Key(_ string) *ValidationResult { + return nil +} + +func (_ *ValidationResult) Message(_ string, _ ...interface{}) *ValidationResult { + return nil +} + +func (_ *ValidationResult) MessageKey(_ string, _ ...interface{}) *ValidationResult { + return nil +} + +type Validator interface { + DefaultMessage() string + IsSatisfied(_ interface{}) bool +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt new file mode 100644 index 00000000000..b42e9ce651d --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt @@ -0,0 +1,42 @@ +# github.com/fsnotify/fsnotify v1.4.9 +## explicit +github.com/fsnotify/fsnotify +# github.com/github/depstubber v0.0.0-20200910105848-4f6b1e61222c +## explicit +github.com/github/depstubber +# github.com/go-stack/stack v1.8.0 +## explicit +github.com/go-stack/stack +# github.com/inconshreveable/log15 v0.0.0-20200109203555-b30bc20e4fd1 +## explicit +github.com/inconshreveable/log15 +# github.com/mattn/go-colorable v0.1.7 +## explicit +github.com/mattn/go-colorable +# github.com/revel/config v1.0.0 +## explicit +github.com/revel/config +# github.com/revel/log15 v2.11.20+incompatible +## explicit +github.com/revel/log15 +# github.com/revel/pathtree v0.0.0-20140121041023-41257a1839e9 +## explicit +github.com/revel/pathtree +# github.com/revel/revel v1.0.0 +## explicit +github.com/revel/revel +# github.com/twinj/uuid v1.0.0 +## explicit +github.com/twinj/uuid +# github.com/xeonx/timeago v1.0.0-rc4 +## explicit +github.com/xeonx/timeago +# golang.org/x/net v0.0.0-20200904194848-62affa334b73 +## explicit +golang.org/x/net +# gopkg.in/natefinch/lumberjack.v2 v2.0.0 +## explicit +gopkg.in/natefinch/lumberjack.v2 +# gopkg.in/stack.v0 v0.0.0-20141108040640-9b43fcefddd0 +## explicit +gopkg.in/stack.v0 From b2b8f104188a92c063ee258931c39ca36e0c542f Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 7 Oct 2020 15:46:39 +0100 Subject: [PATCH 04/19] Fix stub for Revel Embedded fields aren't stubbed correctly --- .../vendor/github.com/revel/revel/stub.go | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go index c2bfe8c15ef..199f1fd7490 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go @@ -1,10 +1,10 @@ -// Code generated by depstubber. DO NOT EDIT. +// Code generated by depstubber. It has been edited to fix a problem with embedded fields. // This is a simple stub for github.com/revel/revel, strictly for use in testing. // See the LICENSE file for information about the licensing of the original library. // Source: github.com/revel/revel (exports: Controller,Params,Request,Router; functions: HTTP_QUERY) -// Package revel is a stub of github.com/revel/revel, generated by depstubber. +// Package revel is a stub of github.com/revel/revel, generated by depstubber, with minor edits. package revel import ( @@ -285,29 +285,15 @@ func (_ *OutResponse) Write(_ []byte) (int, error) { } type Params struct { - Values url.Values - Fixed url.Values - Route url.Values - Query url.Values - Form url.Values - Files map[string][]*multipart.FileHeader - JSON []byte + url.Values + Fixed url.Values + Route url.Values + Query url.Values + Form url.Values + Files map[string][]*multipart.FileHeader + JSON []byte } -func (_ Params) Add(_ string, _ string) {} - -func (_ Params) Del(_ string) {} - -func (_ Params) Encode() string { - return "" -} - -func (_ Params) Get(_ string) string { - return "" -} - -func (_ Params) Set(_ string, _ string) {} - func (_ *Params) Bind(_ interface{}, _ string) {} func (_ *Params) BindJSON(_ interface{}) error { From 9aceae8bd61669cec9b8590d26a8e168ce831513 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 15 Oct 2020 15:27:52 +0100 Subject: [PATCH 05/19] Revel: add support and tests for Render and Redirect sinks. --- .github/workflows/codeqltest.yml | 4 +- change-notes/2020-10-19-revel.md | 2 + ql/src/semmle/go/frameworks/Revel.qll | 115 ++++++++++++++++ ql/src/semmle/go/security/OpenUrlRedirect.qll | 3 + .../OpenUrlRedirectCustomizations.qll | 8 ++ .../security/ReflectedXssCustomizations.qll | 2 + .../semmle/go/frameworks/Revel/EndToEnd.go | 95 +++++++++++++ .../go/frameworks/Revel/OpenRedirect.expected | 13 ++ .../go/frameworks/Revel/OpenRedirect.qlref | 1 + .../go/frameworks/Revel/ReflectedXss.expected | 23 ++++ .../go/frameworks/Revel/ReflectedXss.qlref | 1 + .../go/frameworks/Revel/TaintedPath.expected | 23 ++++ .../go/frameworks/Revel/TaintedPath.qlref | 1 + .../semmle/go/frameworks/Revel/go.mod | 11 +- .../modules/static/app/controllers/stub.go | 130 ++++++++++++++++++ .../vendor/github.com/revel/revel/stub.go | 6 + .../go/frameworks/Revel/vendor/modules.txt | 27 +--- 17 files changed, 430 insertions(+), 35 deletions(-) create mode 100644 change-notes/2020-10-19-revel.md create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/EndToEnd.go create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.qlref create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.qlref create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.expected create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.qlref create mode 100644 ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/modules/static/app/controllers/stub.go diff --git a/.github/workflows/codeqltest.yml b/.github/workflows/codeqltest.yml index 2c3bf09896d..58b4f68e499 100644 --- a/.github/workflows/codeqltest.yml +++ b/.github/workflows/codeqltest.yml @@ -65,7 +65,7 @@ jobs: echo "Done" cd $HOME echo "Downloading CodeQL CLI..." - curl https://github.com/github/codeql-cli-binaries/releases/download/v2.2.5/codeql.zip -L -o codeql.zip + curl https://github.com/github/codeql-cli-binaries/releases/download/v2.3.0/codeql.zip -L -o codeql.zip echo "Done" echo "Unpacking CodeQL CLI..." unzip -q codeql.zip @@ -98,7 +98,7 @@ jobs: echo "Done" cd "$HOME" echo "Downloading CodeQL CLI..." - Invoke-WebRequest -Uri https://github.com/github/codeql-cli-binaries/releases/download/v2.2.5/codeql.zip -OutFile codeql.zip + Invoke-WebRequest -Uri https://github.com/github/codeql-cli-binaries/releases/download/v2.3.0/codeql.zip -OutFile codeql.zip echo "Done" echo "Unpacking CodeQL CLI..." Expand-Archive codeql.zip -DestinationPath $HOME diff --git a/change-notes/2020-10-19-revel.md b/change-notes/2020-10-19-revel.md new file mode 100644 index 00000000000..3683c95ac14 --- /dev/null +++ b/change-notes/2020-10-19-revel.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added basic support for the Revel web framework. diff --git a/ql/src/semmle/go/frameworks/Revel.qll b/ql/src/semmle/go/frameworks/Revel.qll index 365c30fc2ed..4982b0314f8 100644 --- a/ql/src/semmle/go/frameworks/Revel.qll +++ b/ql/src/semmle/go/frameworks/Revel.qll @@ -3,6 +3,7 @@ */ import go +private import semmle.go.security.OpenUrlRedirectCustomizations module Revel { /** Gets the package name. */ @@ -28,6 +29,23 @@ module Revel { } } + /** + * Reinstate the usual field propagation rules for fields, which the OpenURLRedirect + * query usually excludes, for fields of `Params` other than `Params.Fixed`. + */ + private class PropagateParamsFields extends OpenUrlRedirect::AdditionalStep { + PropagateParamsFields() { this = "PropagateParamsFields" } + + override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(Field f, string field | + f.hasQualifiedName(packagePath(), "Params", field) and + field != "Fixed" + | + succ.(Read).readsField(pred, f) + ) + } + } + private class ParamsBind extends TaintTracking::FunctionModel, Method { ParamsBind() { this.hasQualifiedName(packagePath(), "Params", ["Bind", "BindJSON"]) } @@ -94,4 +112,101 @@ module Revel { inp.isReceiver() and outp.isResult(0) } } + + private string contentTypeFromFilename(DataFlow::Node filename) { + if filename.getStringValue().toLowerCase().matches(["%.htm", "%.html"]) + then result = "text/html" + else result = "application/octet-stream" + // Actually Revel can figure out a variety of other content-types, but none of our analyses care to + // distinguish ones other than text/html. + } + + /** + * `revel.Controller` methods which set the response content-type to and designate a result in one operation. + * + * Note these don't actually generate the response, they return a struct which is then returned by the controller + * method, but it is very likely if a string is being rendered that it will end up sent to the user. + * + * The `Render` and `RenderTemplate` methods are excluded for now because both execute HTML templates, and deciding + * whether a particular value is exposed unescaped or not requires parsing the template. + * + * The `RenderError` method can actually return HTML content, but again only via an HTML template if one exists; + * we assume it falls back to return plain text as this implies there is probably not an injection opportunity + * but there is an information leakage issue. + * + * The `RenderBinary` method can also return a variety of content-types based on the file extension passed. + * We look particularly for html file extensions, since these are the only ones we currently have special rules + * for (in particular, detecting XSS vulnerabilities). + */ + private class ControllerRenderMethods extends HTTP::ResponseBody::Range { + string contentType; + + ControllerRenderMethods() { + exists(Method m, string methodName, DataFlow::CallNode methodCall | + m.hasQualifiedName(packagePath(), "Controller", methodName) and + methodCall = m.getACall() + | + exists(int exposedArgument | + this = methodCall.getArgument(exposedArgument) and + ( + methodName = "RenderBinary" and + contentType = contentTypeFromFilename(methodCall.getArgument(1)) and + exposedArgument = 0 + or + methodName = "RenderError" and contentType = "text/plain" and exposedArgument = 0 + or + methodName = "RenderHTML" and contentType = "text/html" and exposedArgument = 0 + or + methodName = "RenderJSON" and contentType = "application/json" and exposedArgument = 0 + or + methodName = "RenderJSONP" and + contentType = "application/javascript" and + exposedArgument = 1 + or + methodName = "RenderXML" and contentType = "text/xml" and exposedArgument = 0 + ) + ) + or + methodName = "RenderText" and + contentType = "text/plain" and + this = methodCall.getAnArgument() + ) + } + + override HTTP::ResponseWriter getResponseWriter() { none() } + + override string getAContentType() { result = contentType } + } + + /** + * The `revel.Controller.RenderFileName` method, which instructs Revel to open a file and return its contents. + * We extend FileSystemAccess rather than HTTP::ResponseBody as this will usually mean exposing a user-controlled + * file rather than the actual contents being user-controlled. + */ + private class IoUtilFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode { + IoUtilFileSystemAccess() { + this = + any(Method m | m.hasQualifiedName(packagePath(), "Controller", "RenderFileName")).getACall() + } + + override DataFlow::Node getAPathArgument() { result = getArgument(0) } + } + + /** + * The `revel.Controller.Redirect` method. + * + * For now I assume that in the context `Redirect(url, value)`, where Revel will `Sprintf(url, value)` internally, + * it is very likely `url` imposes some mandatory prefix, so `value` isn't truly an open redirect opportunity. + */ + private class ControllerRedirectMethod extends HTTP::Redirect::Range, DataFlow::CallNode { + ControllerRedirectMethod() { + exists(Method m | m.hasQualifiedName(packagePath(), "Controller", "Redirect") | + this = m.getACall() + ) + } + + override DataFlow::Node getUrl() { result = this.getArgument(0) } + + override HTTP::ResponseWriter getResponseWriter() { none() } + } } diff --git a/ql/src/semmle/go/security/OpenUrlRedirect.qll b/ql/src/semmle/go/security/OpenUrlRedirect.qll index 4fcc909af52..f2d5210d952 100644 --- a/ql/src/semmle/go/security/OpenUrlRedirect.qll +++ b/ql/src/semmle/go/security/OpenUrlRedirect.qll @@ -33,6 +33,9 @@ module OpenUrlRedirect { // taint steps that do not include flow through fields TaintTracking::localTaintStep(pred, succ) and not TaintTracking::fieldReadStep(pred, succ) or + // explicit extra taint steps for this query + any(AdditionalStep s).hasTaintStep(pred, succ) + or // propagate to a URL when its host is assigned to exists(Write w, Field f, SsaWithFields v | f.hasQualifiedName("net/url", "URL", "Host") | w.writesField(v.getAUse(), f, pred) and succ = v.getAUse() diff --git a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll index 351bdfd7bb2..1bf433103d9 100644 --- a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll +++ b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll @@ -34,6 +34,14 @@ module OpenUrlRedirect { */ abstract class BarrierGuard extends DataFlow::BarrierGuard { } + /** + * An additional taint propagation step specific to this query. + */ + bindingset[this] + abstract class AdditionalStep extends string { + abstract predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ); + } + /** * A source of third-party user input, considered as a flow source for URL redirects. */ diff --git a/ql/src/semmle/go/security/ReflectedXssCustomizations.qll b/ql/src/semmle/go/security/ReflectedXssCustomizations.qll index 4316866020f..cb3ffaaae0a 100644 --- a/ql/src/semmle/go/security/ReflectedXssCustomizations.qll +++ b/ql/src/semmle/go/security/ReflectedXssCustomizations.qll @@ -56,6 +56,8 @@ module ReflectedXss { private predicate nonHtmlContentType(HTTP::ResponseBody body) { not htmlTypeSpecified(body) and ( + exists(body.getAContentType()) + or exists(body.getAContentTypeNode()) or exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") | diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/EndToEnd.go b/ql/test/library-tests/semmle/go/frameworks/Revel/EndToEnd.go new file mode 100644 index 00000000000..5e275412b73 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/EndToEnd.go @@ -0,0 +1,95 @@ +package main + +import ( + "bytes" + "errors" + staticControllers "github.com/revel/modules/static/app/controllers" + "github.com/revel/revel" + "os" + "time" +) + +// Use typical inheritence pattern, per github.com/revel/examples/booking: + +// Doesn't really matter which controller is used here since it'll be stubbed-- +// the important thing is we're inheriting a controller from a Revel module, which +// itself implements revel.Controller. +type MyApplication struct { + staticControllers.Static +} + +// ...then it's common for files to inherit the whole-appliction controller. +type MyRoute struct { + MyApplication +} + +// Implement some request handlers on that Controller exhibiting some common problems: + +func (c MyRoute) Handler1() revel.Result { + // GOOD: the Render function is likely to properly escape the user-controlled parameter. + return c.Render("someviewparam", c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler2() revel.Result { + // BAD: the RenderBinary function copies an `io.Reader` to the user's browser. + buf := &bytes.Buffer{} + buf.WriteString(c.Params.Form.Get("someField")) + return c.RenderBinary(buf, "index.html", revel.Inline, time.Now()) +} + +func (c MyRoute) Handler3() revel.Result { + // GOOD: the RenderBinary function copies an `io.Reader` to the user's browser, but the filename + // means it will be given a safe content-type. + buf := &bytes.Buffer{} + buf.WriteString(c.Params.Form.Get("someField")) + return c.RenderBinary(buf, "index.txt", revel.Inline, time.Now()) +} + +func (c MyRoute) Handler4() revel.Result { + // GOOD: the RenderError function either uses an HTML template with probable escaping, + // or it uses content-type text/plain. + err := errors.New(c.Params.Form.Get("someField")) + return c.RenderError(err) +} + +func (c MyRoute) Handler5() revel.Result { + // BAD: returning an arbitrary file (but this is detected at the os.Open call, not + // due to modelling Revel) + f, _ := os.Open(c.Params.Form.Get("someField")) + return c.RenderFile(f, revel.Inline) +} + +func (c MyRoute) Handler6() revel.Result { + // BAD: returning an arbitrary file (detected as a user-controlled file-op, not XSS) + return c.RenderFileName(c.Params.Form.Get("someField"), revel.Inline) +} + +func (c MyRoute) Handler7() revel.Result { + // BAD: straightforward XSS + return c.RenderHTML(c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler8() revel.Result { + // GOOD: uses JSON content-type + return c.RenderJSON(c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler9() revel.Result { + // GOOD: uses Javascript content-type + return c.RenderJSONP("callback", c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler10() revel.Result { + // GOOD: uses text content-type + return c.RenderText(c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler11() revel.Result { + // GOOD: uses xml content-type + return c.RenderXML(c.Params.Form.Get("someField")) +} + +func (c MyRoute) Handler12() revel.Result { + // BAD: open redirect + return c.Redirect(c.Params.Form.Get("someField")) +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected new file mode 100644 index 00000000000..a98844e8405 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected @@ -0,0 +1,13 @@ +edges +| EndToEnd.go:94:20:94:27 | implicit dereference : Params | EndToEnd.go:94:20:94:27 | implicit dereference : Params | +| EndToEnd.go:94:20:94:27 | implicit dereference : Params | EndToEnd.go:94:20:94:27 | selection of Params : pointer type | +| EndToEnd.go:94:20:94:27 | implicit dereference : Params | EndToEnd.go:94:20:94:49 | call to Get | +| EndToEnd.go:94:20:94:27 | selection of Params : pointer type | EndToEnd.go:94:20:94:27 | implicit dereference : Params | +| EndToEnd.go:94:20:94:27 | selection of Params : pointer type | EndToEnd.go:94:20:94:27 | selection of Params : pointer type | +| EndToEnd.go:94:20:94:27 | selection of Params : pointer type | EndToEnd.go:94:20:94:49 | call to Get | +nodes +| EndToEnd.go:94:20:94:27 | implicit dereference : Params | semmle.label | implicit dereference : Params | +| EndToEnd.go:94:20:94:27 | selection of Params : pointer type | semmle.label | selection of Params : pointer type | +| EndToEnd.go:94:20:94:49 | call to Get | semmle.label | call to Get | +#select +| EndToEnd.go:94:20:94:49 | call to Get | EndToEnd.go:94:20:94:27 | selection of Params : pointer type | EndToEnd.go:94:20:94:49 | call to Get | Untrusted URL redirection due to $@. | EndToEnd.go:94:20:94:27 | selection of Params | user-provided value | diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.qlref b/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.qlref new file mode 100644 index 00000000000..0f1a7477825 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.qlref @@ -0,0 +1 @@ +Security/CWE-601/OpenUrlRedirect.ql diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.expected new file mode 100644 index 00000000000..e6b6e77a9c9 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.expected @@ -0,0 +1,23 @@ +edges +| EndToEnd.go:36:18:36:25 | implicit dereference : Params | EndToEnd.go:36:18:36:25 | implicit dereference : Params | +| EndToEnd.go:36:18:36:25 | implicit dereference : Params | EndToEnd.go:36:18:36:25 | selection of Params : pointer type | +| EndToEnd.go:36:18:36:25 | implicit dereference : Params | EndToEnd.go:37:24:37:26 | buf | +| EndToEnd.go:36:18:36:25 | selection of Params : pointer type | EndToEnd.go:36:18:36:25 | implicit dereference : Params | +| EndToEnd.go:36:18:36:25 | selection of Params : pointer type | EndToEnd.go:36:18:36:25 | selection of Params : pointer type | +| EndToEnd.go:36:18:36:25 | selection of Params : pointer type | EndToEnd.go:37:24:37:26 | buf | +| EndToEnd.go:69:22:69:29 | implicit dereference : Params | EndToEnd.go:69:22:69:29 | implicit dereference : Params | +| EndToEnd.go:69:22:69:29 | implicit dereference : Params | EndToEnd.go:69:22:69:29 | selection of Params : pointer type | +| EndToEnd.go:69:22:69:29 | implicit dereference : Params | EndToEnd.go:69:22:69:51 | call to Get | +| EndToEnd.go:69:22:69:29 | selection of Params : pointer type | EndToEnd.go:69:22:69:29 | implicit dereference : Params | +| EndToEnd.go:69:22:69:29 | selection of Params : pointer type | EndToEnd.go:69:22:69:29 | selection of Params : pointer type | +| EndToEnd.go:69:22:69:29 | selection of Params : pointer type | EndToEnd.go:69:22:69:51 | call to Get | +nodes +| EndToEnd.go:36:18:36:25 | implicit dereference : Params | semmle.label | implicit dereference : Params | +| EndToEnd.go:36:18:36:25 | selection of Params : pointer type | semmle.label | selection of Params : pointer type | +| EndToEnd.go:37:24:37:26 | buf | semmle.label | buf | +| EndToEnd.go:69:22:69:29 | implicit dereference : Params | semmle.label | implicit dereference : Params | +| EndToEnd.go:69:22:69:29 | selection of Params : pointer type | semmle.label | selection of Params : pointer type | +| EndToEnd.go:69:22:69:51 | call to Get | semmle.label | call to Get | +#select +| EndToEnd.go:37:24:37:26 | buf | EndToEnd.go:36:18:36:25 | selection of Params : pointer type | EndToEnd.go:37:24:37:26 | buf | Cross-site scripting vulnerability due to $@. | EndToEnd.go:36:18:36:25 | selection of Params | user-provided value | +| EndToEnd.go:69:22:69:51 | call to Get | EndToEnd.go:69:22:69:29 | selection of Params : pointer type | EndToEnd.go:69:22:69:51 | call to Get | Cross-site scripting vulnerability due to $@. | EndToEnd.go:69:22:69:29 | selection of Params | user-provided value | diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.qlref b/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.qlref new file mode 100644 index 00000000000..e0efe102416 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/ReflectedXss.qlref @@ -0,0 +1 @@ +Security/CWE-079/ReflectedXss.ql diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.expected new file mode 100644 index 00000000000..e7e9830b7eb --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.expected @@ -0,0 +1,23 @@ +edges +| EndToEnd.go:58:18:58:25 | implicit dereference : Params | EndToEnd.go:58:18:58:25 | implicit dereference : Params | +| EndToEnd.go:58:18:58:25 | implicit dereference : Params | EndToEnd.go:58:18:58:25 | selection of Params : pointer type | +| EndToEnd.go:58:18:58:25 | implicit dereference : Params | EndToEnd.go:58:18:58:47 | call to Get | +| EndToEnd.go:58:18:58:25 | selection of Params : pointer type | EndToEnd.go:58:18:58:25 | implicit dereference : Params | +| EndToEnd.go:58:18:58:25 | selection of Params : pointer type | EndToEnd.go:58:18:58:25 | selection of Params : pointer type | +| EndToEnd.go:58:18:58:25 | selection of Params : pointer type | EndToEnd.go:58:18:58:47 | call to Get | +| EndToEnd.go:64:26:64:33 | implicit dereference : Params | EndToEnd.go:64:26:64:33 | implicit dereference : Params | +| EndToEnd.go:64:26:64:33 | implicit dereference : Params | EndToEnd.go:64:26:64:33 | selection of Params : pointer type | +| EndToEnd.go:64:26:64:33 | implicit dereference : Params | EndToEnd.go:64:26:64:55 | call to Get | +| EndToEnd.go:64:26:64:33 | selection of Params : pointer type | EndToEnd.go:64:26:64:33 | implicit dereference : Params | +| EndToEnd.go:64:26:64:33 | selection of Params : pointer type | EndToEnd.go:64:26:64:33 | selection of Params : pointer type | +| EndToEnd.go:64:26:64:33 | selection of Params : pointer type | EndToEnd.go:64:26:64:55 | call to Get | +nodes +| EndToEnd.go:58:18:58:25 | implicit dereference : Params | semmle.label | implicit dereference : Params | +| EndToEnd.go:58:18:58:25 | selection of Params : pointer type | semmle.label | selection of Params : pointer type | +| EndToEnd.go:58:18:58:47 | call to Get | semmle.label | call to Get | +| EndToEnd.go:64:26:64:33 | implicit dereference : Params | semmle.label | implicit dereference : Params | +| EndToEnd.go:64:26:64:33 | selection of Params : pointer type | semmle.label | selection of Params : pointer type | +| EndToEnd.go:64:26:64:55 | call to Get | semmle.label | call to Get | +#select +| EndToEnd.go:58:18:58:47 | call to Get | EndToEnd.go:58:18:58:25 | selection of Params : pointer type | EndToEnd.go:58:18:58:47 | call to Get | This path depends on $@. | EndToEnd.go:58:18:58:25 | selection of Params | a user-provided value | +| EndToEnd.go:64:26:64:55 | call to Get | EndToEnd.go:64:26:64:33 | selection of Params : pointer type | EndToEnd.go:64:26:64:55 | call to Get | This path depends on $@. | EndToEnd.go:64:26:64:33 | selection of Params | a user-provided value | diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.qlref b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.qlref new file mode 100644 index 00000000000..53d53cb8dc5 --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintedPath.qlref @@ -0,0 +1 @@ +Security/CWE-022/TaintedPath.ql diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod b/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod index a0968095036..74524d1d9e1 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/go.mod @@ -3,18 +3,11 @@ module codeql-go-tests/frameworks/Revel go 1.14 require ( - github.com/fsnotify/fsnotify v1.4.9 // indirect - github.com/github/depstubber v0.0.0-20200910105848-4f6b1e61222c // indirect + github.com/github/depstubber v0.0.0-20200916130315-f3217697abd4 // indirect github.com/go-stack/stack v1.8.0 // indirect - github.com/inconshreveable/log15 v0.0.0-20200109203555-b30bc20e4fd1 // indirect github.com/mattn/go-colorable v0.1.7 // indirect github.com/revel/config v1.0.0 // indirect - github.com/revel/log15 v2.11.20+incompatible // indirect - github.com/revel/pathtree v0.0.0-20140121041023-41257a1839e9 // indirect + github.com/revel/modules v1.0.0 github.com/revel/revel v1.0.0 - github.com/twinj/uuid v1.0.0 // indirect - github.com/xeonx/timeago v1.0.0-rc4 // indirect golang.org/x/net v0.0.0-20200904194848-62affa334b73 // indirect - gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect - gopkg.in/stack.v0 v0.0.0-20141108040640-9b43fcefddd0 // indirect ) diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/modules/static/app/controllers/stub.go b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/modules/static/app/controllers/stub.go new file mode 100644 index 00000000000..86e0e1eefbc --- /dev/null +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/modules/static/app/controllers/stub.go @@ -0,0 +1,130 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/revel/modules/static/app/controllers, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/revel/modules/static/app/controllers (exports: Static; functions: ) + +// Package controllers is a stub of github.com/revel/modules/static/app/controllers, generated by depstubber. +package controllers + +import ( + "github.com/revel/revel" +) + +/* +import ( + io "io" + http "net/http" + os "os" + time "time" +) +*/ +type Static struct { + *revel.Controller +} + +/* +func (_ Static) Destroy() {} + +func (_ Static) FlashParams() {} + +func (_ Static) Forbidden(_ string, _ ...interface{}) interface{} { + return nil +} + +func (_ Static) Message(_ string, _ ...interface{}) string { + return "" +} + +func (_ Static) NotFound(_ string, _ ...interface{}) interface{} { + return nil +} + +func (_ Static) Redirect(_ interface{}, _ ...interface{}) interface{} { + return nil +} + +func (_ Static) Render(_ ...interface{}) interface{} { + return nil +} + +func (_ Static) RenderBinary(_ io.Reader, _ string, _ interface{}, _ time.Time) interface{} { + return nil +} + +func (_ Static) RenderError(_ error) interface{} { + return nil +} + +func (_ Static) RenderFile(_ *os.File, _ interface{}) interface{} { + return nil +} + +func (_ Static) RenderFileName(_ string, _ interface{}) interface{} { + return nil +} + +func (_ Static) RenderHTML(_ string) interface{} { + return nil +} + +func (_ Static) RenderJSON(_ interface{}) interface{} { + return nil +} + +func (_ Static) RenderJSONP(_ string, _ interface{}) interface{} { + return nil +} + +func (_ Static) RenderTemplate(_ string) interface{} { + return nil +} + +func (_ Static) RenderText(_ string, _ ...interface{}) interface{} { + return nil +} + +func (_ Static) RenderXML(_ interface{}) interface{} { + return nil +} + +func (_ Static) Serve(_ string, _ string) interface{} { + return nil +} + +func (_ Static) ServeDir(_ string, _ string) interface{} { + return nil +} + +func (_ Static) ServeModule(_ string, _ string, _ string) interface{} { + return nil +} + +func (_ Static) ServeModuleDir(_ string, _ string, _ string) interface{} { + return nil +} + +func (_ Static) SetAction(_ string, _ string) error { + return nil +} + +func (_ Static) SetController(_ interface{}) {} + +func (_ Static) SetCookie(_ *http.Cookie) {} + +func (_ Static) SetTypeAction(_ string, _ string, _ interface{}) error { + return nil +} + +func (_ Static) Stats() map[string]interface{} { + return nil +} + +func (_ Static) TemplateOutput(_ string) ([]byte, error) { + return nil, nil +} + +func (_ Static) Todo() interface{} { + return nil +} +*/ diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go index 199f1fd7490..67461bcdd6f 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/github.com/revel/revel/stub.go @@ -55,6 +55,12 @@ func (_ *ActionDefinition) String() string { type ContentDisposition string +var ( + NoDisposition ContentDisposition = "" + Attachment ContentDisposition = "attachment" + Inline ContentDisposition = "inline" +) + type Controller struct { Name string Type *ControllerType diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt index b42e9ce651d..9e4f72122ca 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/vendor/modules.txt @@ -1,42 +1,21 @@ -# github.com/fsnotify/fsnotify v1.4.9 -## explicit -github.com/fsnotify/fsnotify -# github.com/github/depstubber v0.0.0-20200910105848-4f6b1e61222c +# github.com/github/depstubber v0.0.0-20200916130315-f3217697abd4 ## explicit github.com/github/depstubber # github.com/go-stack/stack v1.8.0 ## explicit github.com/go-stack/stack -# github.com/inconshreveable/log15 v0.0.0-20200109203555-b30bc20e4fd1 -## explicit -github.com/inconshreveable/log15 # github.com/mattn/go-colorable v0.1.7 ## explicit github.com/mattn/go-colorable # github.com/revel/config v1.0.0 ## explicit github.com/revel/config -# github.com/revel/log15 v2.11.20+incompatible +# github.com/revel/modules v1.0.0 ## explicit -github.com/revel/log15 -# github.com/revel/pathtree v0.0.0-20140121041023-41257a1839e9 -## explicit -github.com/revel/pathtree +github.com/revel/modules # github.com/revel/revel v1.0.0 ## explicit github.com/revel/revel -# github.com/twinj/uuid v1.0.0 -## explicit -github.com/twinj/uuid -# github.com/xeonx/timeago v1.0.0-rc4 -## explicit -github.com/xeonx/timeago # golang.org/x/net v0.0.0-20200904194848-62affa334b73 ## explicit golang.org/x/net -# gopkg.in/natefinch/lumberjack.v2 v2.0.0 -## explicit -gopkg.in/natefinch/lumberjack.v2 -# gopkg.in/stack.v0 v0.0.0-20141108040640-9b43fcefddd0 -## explicit -gopkg.in/stack.v0 From e823712adfcb50df1daaf79b8be3114342b6664b Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Wed, 21 Oct 2020 00:48:35 -0700 Subject: [PATCH 06/19] Add utility predicates to FunctionModel Co-authored-by: Chris Smowton --- .../Security/CWE-352/ConstantOauth2State.ql | 2 +- .../go/dataflow/internal/DataFlowUtil.qll | 27 ++++++++++++---- .../dataflow/internal/TaintTrackingUtil.qll | 32 ++++++++++++------- .../go/security/SafeUrlFlowCustomizations.qll | 2 +- .../FunctionModelStep.ql | 4 +-- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/ql/src/Security/CWE-352/ConstantOauth2State.ql b/ql/src/Security/CWE-352/ConstantOauth2State.ql index f00cf749400..d28d6c8474b 100644 --- a/ql/src/Security/CWE-352/ConstantOauth2State.ql +++ b/ql/src/Security/CWE-352/ConstantOauth2State.ql @@ -101,7 +101,7 @@ class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration { TaintTracking::referenceStep(pred, succ) or // Propagate across Sprintf and similar calls - TaintTracking::functionModelStep(any(Fmt::Sprinter s), pred, succ) + any(Fmt::Sprinter s).taintStep(pred, succ) } predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) { diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index 4b7f8e6231e..da941b3efa4 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -896,6 +896,26 @@ class TypeCastNode extends ExprNode { abstract class FunctionModel extends Function { /** Holds if data flows through this function from `input` to `output`. */ abstract predicate hasDataFlow(FunctionInput input, FunctionOutput output); + + /** Gets an input node for this model for the call `c`. */ + DataFlow::Node getAnInputNode(DataFlow::CallNode c) { this.flowStepForCall(result, _, c) } + + /** Gets an output node for this model for the call `c`. */ + DataFlow::Node getAnOutputNode(DataFlow::CallNode c) { this.flowStepForCall(_, result, c) } + + /** Holds if this function model causes data to flow from `pred` to `succ` for the call `c`. */ + predicate flowStepForCall(DataFlow::Node pred, DataFlow::Node succ, DataFlow::CallNode c) { + c = this.getACall() and + exists(FunctionInput inp, FunctionOutput outp | this.hasDataFlow(inp, outp) | + pred = inp.getNode(c) and + succ = outp.getNode(c) + ) + } + + /** Holds if this function model causes data to flow from `pred` to `succ`. */ + predicate flowStep(DataFlow::Node pred, DataFlow::Node succ) { + this.flowStepForCall(pred, succ, _) + } } /** @@ -1006,12 +1026,7 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { basicLocalFlowStep(nodeFrom, nodeTo) or // step through function model - exists(FunctionModel m, CallNode c, FunctionInput inp, FunctionOutput outp | - c = m.getACall() and - m.hasDataFlow(inp, outp) and - nodeFrom = inp.getNode(c) and - nodeTo = outp.getNode(c) - ) + any(FunctionModel m).flowStep(nodeFrom, nodeTo) } /** diff --git a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll index f16b0cc4af5..b05a81bf0c2 100644 --- a/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/TaintTrackingUtil.qll @@ -60,7 +60,7 @@ predicate localAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { tupleStep(pred, succ) or stringConcatStep(pred, succ) or sliceStep(pred, succ) or - functionModelStep(_, pred, succ) or + any(FunctionModel fm).taintStep(pred, succ) or any(AdditionalTaintStep a).step(pred, succ) } @@ -140,16 +140,6 @@ predicate sliceStep(DataFlow::Node pred, DataFlow::Node succ) { succ.(DataFlow::SliceNode).getBase() = pred } -/** Holds if taint flows from `pred` to `succ` via a function model. */ -predicate functionModelStep(FunctionModel fn, DataFlow::Node pred, DataFlow::Node succ) { - exists(DataFlow::CallNode c, FunctionInput inp, FunctionOutput outp | - c = fn.getACall() and - fn.hasTaintFlow(inp, outp) and - pred = inp.getNode(c) and - succ = outp.getNode(c) - ) -} - /** * A model of a function specifying that the function propagates taint from * a parameter or qualifier to a result. @@ -157,6 +147,26 @@ predicate functionModelStep(FunctionModel fn, DataFlow::Node pred, DataFlow::Nod abstract class FunctionModel extends Function { /** Holds if taint propagates through this function from `input` to `output`. */ abstract predicate hasTaintFlow(FunctionInput input, FunctionOutput output); + + /** Gets an input node for this model for the call `c`. */ + DataFlow::Node getAnInputNode(DataFlow::CallNode c) { this.taintStepForCall(result, _, c) } + + /** Gets an output node for this model for the call `c`. */ + DataFlow::Node getAnOutputNode(DataFlow::CallNode c) { this.taintStepForCall(_, result, c) } + + /** Holds if this function model causes taint to flow from `pred` to `succ` for the call `c`. */ + predicate taintStepForCall(DataFlow::Node pred, DataFlow::Node succ, DataFlow::CallNode c) { + c = this.getACall() and + exists(FunctionInput inp, FunctionOutput outp | this.hasTaintFlow(inp, outp) | + pred = inp.getNode(c) and + succ = outp.getNode(c) + ) + } + + /** Holds if this function model causes taint to flow from `pred` to `succ`. */ + predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { + this.taintStepForCall(pred, succ, _) + } } /** diff --git a/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll b/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll index c73aae7f623..6b53df3e6d2 100644 --- a/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll +++ b/ql/src/semmle/go/security/SafeUrlFlowCustomizations.qll @@ -25,6 +25,6 @@ module SafeUrlFlow { /** A function model step using `UnsafeUrlMethod`, considered as a sanitizer for safe URL flow. */ private class UnsafeUrlMethodEdge extends SanitizerEdge { - UnsafeUrlMethodEdge() { TaintTracking::functionModelStep(any(UnsafeUrlMethod um), this, _) } + UnsafeUrlMethodEdge() { this = any(UnsafeUrlMethod um).getAnInputNode(_) } } } diff --git a/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql b/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql index 276239350d7..80bfb92cbfa 100644 --- a/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql +++ b/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionModelStep.ql @@ -16,6 +16,6 @@ class ReaderReset extends TaintTracking::FunctionModel, Method { } } -from Function fn, DataFlow::Node pred, DataFlow::Node succ -where TaintTracking::functionModelStep(fn, pred, succ) +from TaintTracking::FunctionModel fn, DataFlow::Node pred, DataFlow::Node succ +where fn.taintStep(pred, succ) select fn, pred, succ From 2818da4df936831fb9dca366e7b79cfd05f892ba Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 21 Oct 2020 17:27:18 +0100 Subject: [PATCH 07/19] Advance to latest codeql-cli release --- .github/workflows/codeqltest.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/codeqltest.yml b/.github/workflows/codeqltest.yml index 58b4f68e499..e9eb778c546 100644 --- a/.github/workflows/codeqltest.yml +++ b/.github/workflows/codeqltest.yml @@ -20,7 +20,7 @@ jobs: echo "Done" cd $HOME echo "Downloading CodeQL CLI..." - curl https://github.com/github/codeql-cli-binaries/releases/download/v2.3.0/codeql.zip -L -o codeql.zip + curl https://github.com/github/codeql-cli-binaries/releases/download/v2.3.1/codeql.zip -L -o codeql.zip echo "Done" echo "Unpacking CodeQL CLI..." unzip -q codeql.zip @@ -65,7 +65,7 @@ jobs: echo "Done" cd $HOME echo "Downloading CodeQL CLI..." - curl https://github.com/github/codeql-cli-binaries/releases/download/v2.3.0/codeql.zip -L -o codeql.zip + curl https://github.com/github/codeql-cli-binaries/releases/download/v2.3.1/codeql.zip -L -o codeql.zip echo "Done" echo "Unpacking CodeQL CLI..." unzip -q codeql.zip @@ -98,7 +98,7 @@ jobs: echo "Done" cd "$HOME" echo "Downloading CodeQL CLI..." - Invoke-WebRequest -Uri https://github.com/github/codeql-cli-binaries/releases/download/v2.3.0/codeql.zip -OutFile codeql.zip + Invoke-WebRequest -Uri https://github.com/github/codeql-cli-binaries/releases/download/v2.3.1/codeql.zip -OutFile codeql.zip echo "Done" echo "Unpacking CodeQL CLI..." Expand-Archive codeql.zip -DestinationPath $HOME From 62c6b0dc3708f11a1080ce30a74b2dbfb7cbdffe Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 21 Oct 2020 17:28:00 +0100 Subject: [PATCH 08/19] Add support for more Revel untrusted sources --- ql/src/semmle/go/frameworks/Revel.qll | 43 ++++++++----------- .../semmle/go/frameworks/Revel/Revel.go | 29 ++++++++++--- .../go/frameworks/Revel/TaintFlows.expected | 29 ++++++++----- 3 files changed, 59 insertions(+), 42 deletions(-) diff --git a/ql/src/semmle/go/frameworks/Revel.qll b/ql/src/semmle/go/frameworks/Revel.qll index 4982b0314f8..878d5ff46f8 100644 --- a/ql/src/semmle/go/frameworks/Revel.qll +++ b/ql/src/semmle/go/frameworks/Revel.qll @@ -70,7 +70,8 @@ module Revel { exists(string fieldName | this.getField().hasQualifiedName(packagePath(), "Request", fieldName) | - fieldName in ["In", "Header", "URL", "Form", "MultipartForm"] + fieldName in ["Header", "ContentType", "AcceptLanguages", "Locale", "URL", "Form", + "MultipartForm"] ) } } @@ -81,13 +82,23 @@ module Revel { this .getTarget() .hasQualifiedName(packagePath(), "Request", - ["FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody"]) + ["FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody", + "Cookie", "GetHttpHeader", "GetRequestURI", "MultipartReader", "Referer", + "UserAgent"]) + } + } + + private class ServerCookieGetValue extends TaintTracking::FunctionModel, Method { + ServerCookieGetValue() { this.hasQualifiedName(packagePath(), "ServerCookie", "GetValue") } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp.isReceiver() and outp.isResult() } } private class ServerMultipartFormGetFiles extends TaintTracking::FunctionModel, Method { ServerMultipartFormGetFiles() { - this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetFiles") + this.hasQualifiedName(packagePath(), "ServerMultipartForm", ["GetFiles", "GetValues"]) } override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { @@ -95,24 +106,6 @@ module Revel { } } - private class ServerMultipartFormGetValues extends TaintTracking::FunctionModel, Method { - ServerMultipartFormGetValues() { - this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetValues") - } - - override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { - inp.isReceiver() and outp.isResult() - } - } - - private class ServerRequestGet extends TaintTracking::FunctionModel, Method { - ServerRequestGet() { this.hasQualifiedName(packagePath(), "ServerRequest", "Get") } - - override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { - inp.isReceiver() and outp.isResult(0) - } - } - private string contentTypeFromFilename(DataFlow::Node filename) { if filename.getStringValue().toLowerCase().matches(["%.htm", "%.html"]) then result = "text/html" @@ -183,8 +176,8 @@ module Revel { * We extend FileSystemAccess rather than HTTP::ResponseBody as this will usually mean exposing a user-controlled * file rather than the actual contents being user-controlled. */ - private class IoUtilFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode { - IoUtilFileSystemAccess() { + private class RenderFileNameCall extends FileSystemAccess::Range, DataFlow::CallNode { + RenderFileNameCall() { this = any(Method m | m.hasQualifiedName(packagePath(), "Controller", "RenderFileName")).getACall() } @@ -195,8 +188,8 @@ module Revel { /** * The `revel.Controller.Redirect` method. * - * For now I assume that in the context `Redirect(url, value)`, where Revel will `Sprintf(url, value)` internally, - * it is very likely `url` imposes some mandatory prefix, so `value` isn't truly an open redirect opportunity. + * It is currently assumed that a tainted `value` in `Redirect(url, value)`, which calls `Sprintf(url, value)` + * internally, cannot lead to an open redirect vulnerability. */ private class ControllerRedirectMethod extends HTTP::Redirect::Range, DataFlow::CallNode { ControllerRedirectMethod() { diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go index b067d0238e5..ae59e7d1f51 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go @@ -78,9 +78,12 @@ func (c myAppController) accessingParamsJSONIsUnsafe() { } func accessingRequestDirectlyIsUnsafe(c *revel.Controller) { - useURLValues(c.Request.GetQuery()) // NOT OK - useURLValues(c.Request.Form) // NOT OK - useURLValues(c.Request.MultipartForm.Value) // NOT OK + useURLValues(c.Request.GetQuery()) // NOT OK + useURLValues(c.Request.Form) // NOT OK + useURLValues(c.Request.MultipartForm.Value) // NOT OK + useString(c.Request.ContentType) // NOT OK + useString(c.Request.AcceptLanguages[0].Language) // NOT OK + useString(c.Request.Locale) // NOT OK form, _ := c.Request.GetForm() // NOT OK useURLValues(form) @@ -95,12 +98,26 @@ func accessingRequestDirectlyIsUnsafe(c *revel.Controller) { json, _ := ioutil.ReadAll(c.Request.GetBody()) // NOT OK useJSON(json) + + cookie, _ := c.Request.Cookie("abc") + useString(cookie.GetValue()) // NOT OK + + useString(c.Request.GetHttpHeader("headername")) // NOT OK + + useString(c.Request.GetRequestURI()) // NOT OK + + reader, _ := c.Request.MultipartReader() + part, _ := reader.NextPart() + partbody := make([]byte, 100) + part.Read(partbody) + useString(string(partbody)) // NOT OK + + useString(c.Request.Referer()) // NOT OK + + useString(c.Request.UserAgent()) // NOT OK } func accessingServerRequest(c *revel.Controller) { - query, _ := c.Request.In.Get(revel.HTTP_QUERY) // NOT OK - useURLValues(query.(url.Values)) - var message string c.Request.WebSocket.MessageReceive(&message) // NOT OK useString(message) diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected index 1b58fbfb755..51be68608b2 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected @@ -21,14 +21,21 @@ | Revel.go:82:15:82:28 | selection of Form : Values | Revel.go:33:12:33:23 | call to Get | 82 | | Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:32:12:32:22 | index expression | 83 | | Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:33:12:33:23 | call to Get | 83 | -| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:32:12:32:22 | index expression | 85 | -| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:33:12:33:23 | call to Get | 85 | -| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:32:12:32:22 | index expression | 88 | -| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:33:12:33:23 | call to Get | 88 | -| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:92:11:92:35 | index expression | 91 | -| Revel.go:94:11:94:33 | selection of MultipartForm : pointer type | Revel.go:94:11:94:48 | index expression | 94 | -| Revel.go:96:28:96:46 | call to GetBody : Reader | Revel.go:97:10:97:13 | json | 96 | -| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:32:12:32:22 | index expression | 101 | -| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:33:12:33:23 | call to Get | 101 | -| Revel.go:105:37:105:44 | &... : pointer type | Revel.go:106:12:106:18 | message | 105 | -| Revel.go:109:41:109:42 | &... : pointer type | Revel.go:110:12:110:12 | p | 109 | +| Revel.go:84:12:84:32 | selection of ContentType | Revel.go:84:12:84:32 | selection of ContentType | 84 | +| Revel.go:85:12:85:36 | selection of AcceptLanguages : AcceptLanguages | Revel.go:85:12:85:48 | selection of Language | 85 | +| Revel.go:86:12:86:27 | selection of Locale | Revel.go:86:12:86:27 | selection of Locale | 86 | +| Revel.go:88:13:88:31 | call to GetForm : tuple type | Revel.go:32:12:32:22 | index expression | 88 | +| Revel.go:88:13:88:31 | call to GetForm : tuple type | Revel.go:33:12:33:23 | call to Get | 88 | +| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:32:12:32:22 | index expression | 91 | +| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:33:12:33:23 | call to Get | 91 | +| Revel.go:94:13:94:40 | call to GetMultipartForm : tuple type | Revel.go:95:11:95:35 | index expression | 94 | +| Revel.go:97:11:97:33 | selection of MultipartForm : pointer type | Revel.go:97:11:97:48 | index expression | 97 | +| Revel.go:99:28:99:46 | call to GetBody : Reader | Revel.go:100:10:100:13 | json | 99 | +| Revel.go:102:15:102:37 | call to Cookie : tuple type | Revel.go:103:12:103:28 | call to GetValue | 102 | +| Revel.go:105:12:105:48 | call to GetHttpHeader | Revel.go:105:12:105:48 | call to GetHttpHeader | 105 | +| Revel.go:107:12:107:36 | call to GetRequestURI | Revel.go:107:12:107:36 | call to GetRequestURI | 107 | +| Revel.go:109:15:109:41 | call to MultipartReader : tuple type | Revel.go:113:12:113:27 | type conversion | 109 | +| Revel.go:115:12:115:30 | call to Referer | Revel.go:115:12:115:30 | call to Referer | 115 | +| Revel.go:117:12:117:32 | call to UserAgent | Revel.go:117:12:117:32 | call to UserAgent | 117 | +| Revel.go:122:37:122:44 | &... : pointer type | Revel.go:123:12:123:18 | message | 122 | +| Revel.go:126:41:126:42 | &... : pointer type | Revel.go:127:12:127:12 | p | 126 | From 5cc695f1d5af1ee3aeb4cba5eb70c79784fed6e4 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 22 Oct 2020 13:19:55 +0100 Subject: [PATCH 09/19] Autobuilder: fall back when os.Executable fails This can happen under tracing, perhaps because of https://github.com/github/codeql-tracer/issues/29 --- .../cli/go-autobuilder/go-autobuilder.go | 54 ++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/extractor/cli/go-autobuilder/go-autobuilder.go b/extractor/cli/go-autobuilder/go-autobuilder.go index 4344187cbd6..8cdd0adeff9 100644 --- a/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/extractor/cli/go-autobuilder/go-autobuilder.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "golang.org/x/mod/semver" "io/ioutil" @@ -206,6 +207,51 @@ func checkVendor() bool { return true } +func getOsToolsSubdir() (string, error) { + switch runtime.GOOS { + case "darwin": + return "osx64", nil + case "linux": + return "linux64", nil + case "windows": + return "win64", nil + } + return "", errors.New("Unknown OS: " + runtime.GOOS) +} + +func getExtractorDir() (string, error) { + mypath, err := os.Executable() + if err == nil { + return filepath.Dir(mypath), nil + } + log.Printf("Could not determine path of autobuilder: %v.\n", err) + + // Fall back to rebuilding our own path from the extractor root: + extractorRoot := os.Getenv("CODEQL_EXTRACTOR_GO_ROOT") + if extractorRoot == "" { + return "", errors.New("CODEQL_EXTRACTOR_GO_ROOT not set") + } + + osSubdir, err := getOsToolsSubdir() + if err != nil { + return "", err + } + + return filepath.Join(extractorRoot, "tools", osSubdir), nil +} + +func getExtractorPath() (string, error) { + dirname, err := getExtractorDir() + if err != nil { + return "", err + } + extractor := filepath.Join(dirname, "go-extractor") + if runtime.GOOS == "windows" { + extractor = extractor + ".exe" + } + return extractor, nil +} + func main() { if len(os.Args) > 1 { usage() @@ -507,13 +553,9 @@ func main() { } // extract - mypath, err := os.Executable() + extractor, err := getExtractorPath() if err != nil { - log.Fatalf("Could not determine path of autobuilder: %v.\n", err) - } - extractor := filepath.Join(filepath.Dir(mypath), "go-extractor") - if runtime.GOOS == "windows" { - extractor = extractor + ".exe" + log.Fatalf("Could not determine path of extractor: %v.\n", err) } cwd, err := os.Getwd() From e22bf96ba31eed9539cb11b4cfa7567f650ac876 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 22 Oct 2020 05:22:33 -0700 Subject: [PATCH 10/19] extractor: Extract the working directory if no packages are passed --- extractor/cli/go-extractor/go-extractor.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/extractor/cli/go-extractor/go-extractor.go b/extractor/cli/go-extractor/go-extractor.go index 52fd8ede692..c7e759d2834 100644 --- a/extractor/cli/go-extractor/go-extractor.go +++ b/extractor/cli/go-extractor/go-extractor.go @@ -108,13 +108,14 @@ func main() { } if len(patterns) == 0 { - log.Println("Nothing to extract.") - } else { - log.Printf("Build flags: '%s'; patterns: '%s'\n", strings.Join(buildFlags, " "), strings.Join(patterns, " ")) - err := extractor.ExtractWithFlags(buildFlags, patterns) - if err != nil { - log.Fatalf("Error running go tooling: %s\n", err.Error()) - } + log.Println("No packages explicitly provided, adding '.'") + patterns = []string{"."} + } + + log.Printf("Build flags: '%s'; patterns: '%s'\n", strings.Join(buildFlags, " "), strings.Join(patterns, " ")) + err := extractor.ExtractWithFlags(buildFlags, patterns) + if err != nil { + log.Fatalf("Error running go tooling: %s\n", err.Error()) } if memprofile != "" { From ec52bdd536464e38a764627bc1bfa80cbe866379 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 22 Oct 2020 06:07:15 -0700 Subject: [PATCH 11/19] Enable code scanning --- .github/workflows/codeql-analysis.yml | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 00000000000..8924efc6d21 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,33 @@ +name: "CodeQL" + +on: + push: + branches: [main, lgtm.com, rc/1.21, rc/1.23, rc/1.24, rc/1.25] + pull_request: + # The branches below must be a subset of the branches above + branches: [main] + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + language: ['go'] + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + + - run: make + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 From 3716f6d7e9765451c80cb3e92c0825e22a3b136f Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 22 Oct 2020 14:42:23 +0100 Subject: [PATCH 12/19] Improve error messages --- extractor/cli/go-autobuilder/go-autobuilder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extractor/cli/go-autobuilder/go-autobuilder.go b/extractor/cli/go-autobuilder/go-autobuilder.go index 8cdd0adeff9..8a2b4a67f6a 100644 --- a/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/extractor/cli/go-autobuilder/go-autobuilder.go @@ -216,7 +216,7 @@ func getOsToolsSubdir() (string, error) { case "windows": return "win64", nil } - return "", errors.New("Unknown OS: " + runtime.GOOS) + return "", errors.New("Unsupported OS: " + runtime.GOOS) } func getExtractorDir() (string, error) { @@ -229,7 +229,7 @@ func getExtractorDir() (string, error) { // Fall back to rebuilding our own path from the extractor root: extractorRoot := os.Getenv("CODEQL_EXTRACTOR_GO_ROOT") if extractorRoot == "" { - return "", errors.New("CODEQL_EXTRACTOR_GO_ROOT not set") + return "", errors.New("CODEQL_EXTRACTOR_GO_ROOT not set.\nThis binary should not be run manually; instead, use the CodeQL CLI or VSCode extension. See https://securitylab.github.com/tools/codeql") } osSubdir, err := getOsToolsSubdir() From 1e034a1dd5aac4d395f40bc34bb6d251d1d8a1bf Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 22 Oct 2020 06:35:24 -0700 Subject: [PATCH 13/19] Add logrus to go.qll --- ql/src/go.qll | 1 + .../Security/CWE-312/CleartextLogging.expected | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/ql/src/go.qll b/ql/src/go.qll index b4b07eff90f..5736153621c 100644 --- a/ql/src/go.qll +++ b/ql/src/go.qll @@ -34,6 +34,7 @@ import semmle.go.frameworks.Email import semmle.go.frameworks.Encoding import semmle.go.frameworks.Gin import semmle.go.frameworks.Glog +import semmle.go.frameworks.Logrus import semmle.go.frameworks.HTTP import semmle.go.frameworks.Macaron import semmle.go.frameworks.Mux diff --git a/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected b/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected index 1778719b87d..f4ebc0e81cd 100644 --- a/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected +++ b/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected @@ -1,6 +1,7 @@ edges | klog.go:20:30:20:37 | selection of Header : Header | klog.go:22:15:22:20 | header | | klog.go:28:13:28:20 | selection of Header : Header | klog.go:28:13:28:41 | call to Get | +| main.go:21:19:21:26 | password : string | main.go:22:29:22:34 | fields | | overrides.go:9:9:9:16 | password : string | overrides.go:13:14:13:23 | call to String | | passwords.go:8:12:8:12 | definition of x : string | passwords.go:9:14:9:14 | x | | passwords.go:30:8:30:15 | password : string | passwords.go:8:12:8:12 | definition of x : string | @@ -31,6 +32,10 @@ nodes | klog.go:28:13:28:41 | call to Get | semmle.label | call to Get | | main.go:15:14:15:21 | password | semmle.label | password | | main.go:17:12:17:19 | password | semmle.label | password | +| main.go:18:17:18:24 | password | semmle.label | password | +| main.go:21:19:21:26 | password : string | semmle.label | password : string | +| main.go:22:29:22:34 | fields | semmle.label | fields | +| main.go:25:35:25:42 | password | semmle.label | password | | overrides.go:9:9:9:16 | password : string | semmle.label | password : string | | overrides.go:13:14:13:23 | call to String | semmle.label | call to String | | passwords.go:8:12:8:12 | definition of x : string | semmle.label | definition of x : string | @@ -78,6 +83,9 @@ nodes | klog.go:28:13:28:41 | call to Get | klog.go:28:13:28:20 | selection of Header : Header | klog.go:28:13:28:41 | call to Get | Sensitive data returned by $@ is logged here. | klog.go:28:13:28:20 | selection of Header | HTTP request headers | | main.go:15:14:15:21 | password | main.go:15:14:15:21 | password | main.go:15:14:15:21 | password | Sensitive data returned by $@ is logged here. | main.go:15:14:15:21 | password | an access to password | | main.go:17:12:17:19 | password | main.go:17:12:17:19 | password | main.go:17:12:17:19 | password | Sensitive data returned by $@ is logged here. | main.go:17:12:17:19 | password | an access to password | +| main.go:18:17:18:24 | password | main.go:18:17:18:24 | password | main.go:18:17:18:24 | password | Sensitive data returned by $@ is logged here. | main.go:18:17:18:24 | password | an access to password | +| main.go:22:29:22:34 | fields | main.go:21:19:21:26 | password : string | main.go:22:29:22:34 | fields | Sensitive data returned by $@ is logged here. | main.go:21:19:21:26 | password | an access to password | +| main.go:25:35:25:42 | password | main.go:25:35:25:42 | password | main.go:25:35:25:42 | password | Sensitive data returned by $@ is logged here. | main.go:25:35:25:42 | password | an access to password | | overrides.go:13:14:13:23 | call to String | overrides.go:9:9:9:16 | password : string | overrides.go:13:14:13:23 | call to String | Sensitive data returned by $@ is logged here. | overrides.go:9:9:9:16 | password | an access to password | | passwords.go:9:14:9:14 | x | passwords.go:30:8:30:15 | password : string | passwords.go:9:14:9:14 | x | Sensitive data returned by $@ is logged here. | passwords.go:30:8:30:15 | password | an access to password | | passwords.go:25:14:25:21 | password | passwords.go:25:14:25:21 | password | passwords.go:25:14:25:21 | password | Sensitive data returned by $@ is logged here. | passwords.go:25:14:25:21 | password | an access to password | From 671b427e1ea1fd13a22fc4ffaadcb3e4d5b5a33a Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 22 Oct 2020 06:41:36 -0700 Subject: [PATCH 14/19] Add shared testing framework It has been modified to use `hasLocation` instead of `Location` --- .../TestUtilities/InlineExpectationsTest.qll | 294 ++++++++++++++++++ .../InlineExpectationsTestPrivate.qll | 10 + 2 files changed, 304 insertions(+) create mode 100644 ql/test/TestUtilities/InlineExpectationsTest.qll create mode 100644 ql/test/TestUtilities/InlineExpectationsTestPrivate.qll diff --git a/ql/test/TestUtilities/InlineExpectationsTest.qll b/ql/test/TestUtilities/InlineExpectationsTest.qll new file mode 100644 index 00000000000..befbde4afc7 --- /dev/null +++ b/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -0,0 +1,294 @@ +/** + * Provides a library for writing QL tests whose success or failure is based on expected results + * embedded in the test source code as comments, rather than a `.expected` file. + * + * To add this framework to a new language: + * - Add a file `InlineExpectationsTestPrivate.qll` that defines a `ExpectationComment` class. This class + * must support a `getContents` method that returns the contents of the given comment, _excluding_ + * the comment indicator itself. It should also define `toString` and `getLocation` as usual. + * + * To create a new inline expectations test: + * - Declare a class that extends `InlineExpectationsTest`. In the characteristic predicate of the + * new class, bind `this` to a unique string (usually the name of the test). + * - Override the `hasActualResult()` predicate to produce the actual results of the query. For each + * result, specify a `Location`, a text description of the element for which the result was + * reported, a short string to serve as the tag to identify expected results for this test, and the + * expected value of the result. + * - Override `getARelevantTag()` to return the set of tags that can be produced by + * `hasActualResult()`. Often this is just a single tag. + * + * Example: + * ```ql + * class ConstantValueTest extends InlineExpectationsTest { + * ConstantValueTest() { this = "ConstantValueTest" } + * + * override string getARelevantTag() { + * // We only use one tag for this test. + * result = "const" + * } + * + * override predicate hasActualResult( + * Location location, string element, string tag, string value + * ) { + * exists(Expr e | + * tag = "const" and // The tag for this test. + * value = e.getValue() and // The expected value. Will only hold for constant expressions. + * location = e.getLocation() and // The location of the result to be reported. + * element = e.toString() // The display text for the result. + * ) + * } + * } + * ``` + * + * There is no need to write a `select` clause or query predicate. All of the differences between + * expected results and actual results will be reported in the `failures()` query predicate. + * + * To annotate the test source code with an expected result, place a comment on the + * same line as the expected result, with text of the following format as the body of the comment: + * + * `$tag=expected-value` + * + * Where `tag` is the value of the `tag` parameter from `hasActualResult()`, and `expected-value` is + * the value of the `value` parameter from `hasActualResult()`. The `=expected-value` portion may be + * omitted, in which case `expected-value` is treated as the empty string. Multiple expectations may + * be placed in the same comment, as long as each is prefixed by a `$`. Any actual result that + * appears on a line that does not contain a matching expected result comment will be reported with + * a message of the form "Unexpected result: tag=value". Any expected result comment for which there + * is no matching actual result will be reported with a message of the form + * "Missing result: tag=expected-value". + * + * Example: + * ```cpp + * int i = x + 5; // $const=5 + * int j = y + (7 - 3) // $const=7 $const=3 $const=4 // The result of the subtraction is a constant. + * ``` + * + * For tests that contain known false positives and false negatives, it is possible to further + * annotate that a particular expected result is known to be a false positive, or that a particular + * missing result is known to be a false negative: + * + * `$f+:tag=expected-value` // False positive + * `$f-:tag=expected-value` // False negative + * + * A false positive expectation is treated as any other expected result, except that if there is no + * matching actual result, the message will be of the form "Fixed false positive: tag=value". A + * false negative expectation is treated as if there were no expected result, except that if a + * matching expected result is found, the message will be of the form + * "Fixed false negative: tag=value". + * + * If the same result value is expected for two or more tags on the same line, there is a shorthand + * notation available: + * + * `$tag1,tag2=expected-value` + * + * is equivalent to: + * + * `$tag1=expected-value $tag2=expected-value` + */ + +private import InlineExpectationsTestPrivate + +/** + * Base class for tests with inline expectations. The test extends this class to provide the actual + * results of the query, which are then compared with the expected results in comments to produce a + * list of failure messages that point out where the actual results differ from the expected + * results. + */ +abstract class InlineExpectationsTest extends string { + bindingset[this] + InlineExpectationsTest() { any() } + + /** + * Returns all tags that can be generated by this test. Most tests will only ever produce a single + * tag. Any expected result comments for a tag that is not returned by the `getARelevantTag()` + * predicate for an active test will be ignored. This makes it possible to write multiple tests in + * different `.ql` files that all query the same source code. + */ + abstract string getARelevantTag(); + + /** + * Returns the actual results of the query that is being tested. Each result consist of the + * following values: + * - `location` - The source code location of the result. Any expected result comment must appear + * on the start line of this location. + * - `element` - Display text for the element on which the result is reported. + * - `tag` - The tag that marks this result as coming from this test. This must be one of the tags + * returned by `getARelevantTag()`. + * - `value` - The value of the result, which will be matched against the value associated with + * `tag` in any expected result comment on that line. + */ + abstract predicate hasActualResult(string file, int line, string element, string tag, string value); + + final predicate hasFailureMessage(FailureLocatable element, string message) { + exists(ActualResult actualResult | + actualResult.getTest() = this and + element = actualResult and + ( + exists(FalseNegativeExpectation falseNegative | + falseNegative.matchesActualResult(actualResult) and + message = "Fixed false negative:" + falseNegative.getExpectationText() + ) + or + not exists(ValidExpectation expectation | expectation.matchesActualResult(actualResult)) and + message = "Unexpected result: " + actualResult.getExpectationText() + ) + ) + or + exists(ValidExpectation expectation | + not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and + expectation.getTag() = getARelevantTag() and + element = expectation and + ( + expectation instanceof GoodExpectation and + message = "Missing result:" + expectation.getExpectationText() + or + expectation instanceof FalsePositiveExpectation and + message = "Fixed false positive:" + expectation.getExpectationText() + ) + ) + or + exists(InvalidExpectation expectation | + element = expectation and + message = "Invalid expectation syntax: " + expectation.getExpectation() + ) + } +} + +/** + * RegEx pattern to match a comment containing one or more expected results. The comment must have + * `$` as its first non-whitespace character. Any subsequent character + * is treated as part of the expected results, except that the comment may contain a `//` sequence + * to treat the remainder of the line as a regular (non-interpreted) comment. + */ +private string expectationCommentPattern() { result = "\\s*(\\$(?:[^/]|/[^/])*)(?://.*)?" } + +/** + * RegEx pattern to match a single expected result, not including the leading `$`. It starts with an + * optional `f+:` or `f-:`, followed by one or more comma-separated tags containing only letters, + * `-`, and `_`, optionally followed by `=` and the expected value. + */ +private string expectationPattern() { + result = "(?:(f(?:\\+|-)):)?((?:[A-Za-z-_]+)(?:\\s*,\\s*[A-Za-z-_]+)*)(?:=(.*))?" +} + +private string getAnExpectation(ExpectationComment comment) { + result = comment.getContents().regexpCapture(expectationCommentPattern(), 1).splitAt("$").trim() and + result != "" +} + +private newtype TFailureLocatable = + TActualResult( + InlineExpectationsTest test, string file, int line, string element, string tag, string value + ) { + test.hasActualResult(file, line, element, tag, value) + } or + TValidExpectation(ExpectationComment comment, string tag, string value, string knownFailure) { + exists(string expectation | + expectation = getAnExpectation(comment) and + expectation.regexpMatch(expectationPattern()) and + tag = expectation.regexpCapture(expectationPattern(), 2).splitAt(",").trim() and + ( + if exists(expectation.regexpCapture(expectationPattern(), 3)) + then value = expectation.regexpCapture(expectationPattern(), 3) + else value = "" + ) and + ( + if exists(expectation.regexpCapture(expectationPattern(), 1)) + then knownFailure = expectation.regexpCapture(expectationPattern(), 1) + else knownFailure = "" + ) + ) + } or + TInvalidExpectation(ExpectationComment comment, string expectation) { + expectation = getAnExpectation(comment) and + not expectation.regexpMatch(expectationPattern()) + } + +class FailureLocatable extends TFailureLocatable { + string toString() { none() } + + predicate hasLocation(string file, int line) { none() } + + final string getExpectationText() { result = getTag() + "=" + getValue() } + + string getTag() { none() } + + string getValue() { none() } +} + +class ActualResult extends FailureLocatable, TActualResult { + InlineExpectationsTest test; + string file; + int line; + string element; + string tag; + string value; + + ActualResult() { this = TActualResult(test, file, line, element, tag, value) } + + override string toString() { result = element } + + override predicate hasLocation(string f, int l) { f = file and l = line } + + InlineExpectationsTest getTest() { result = test } + + override string getTag() { result = tag } + + override string getValue() { result = value } +} + +abstract private class Expectation extends FailureLocatable { + ExpectationComment comment; + + override string toString() { result = comment.toString() } + + override predicate hasLocation(string file, int line) { comment.hasLocationInfo(file, line, _, _, _) } +} + +private class ValidExpectation extends Expectation, TValidExpectation { + string tag; + string value; + string knownFailure; + + ValidExpectation() { this = TValidExpectation(comment, tag, value, knownFailure) } + + override string getTag() { result = tag } + + override string getValue() { result = value } + + string getKnownFailure() { result = knownFailure } + + predicate matchesActualResult(ActualResult actualResult) { + exists(string file, int line | actualResult.hasLocation(file, line) | + this.hasLocation(file, line) + ) and + getTag() = actualResult.getTag() and + getValue() = actualResult.getValue() + } +} + +class GoodExpectation extends ValidExpectation { + GoodExpectation() { getKnownFailure() = "" } +} + +class FalsePositiveExpectation extends ValidExpectation { + FalsePositiveExpectation() { getKnownFailure() = "f+" } +} + +class FalseNegativeExpectation extends ValidExpectation { + FalseNegativeExpectation() { getKnownFailure() = "f-" } +} + +class InvalidExpectation extends Expectation, TInvalidExpectation { + string expectation; + + InvalidExpectation() { this = TInvalidExpectation(comment, expectation) } + + string getExpectation() { result = expectation } +} + +query predicate failures(string file, int line, FailureLocatable element, string message) { + exists(InlineExpectationsTest test | test.hasFailureMessage(element, message) | + element.hasLocation(file, line) + ) +} diff --git a/ql/test/TestUtilities/InlineExpectationsTestPrivate.qll b/ql/test/TestUtilities/InlineExpectationsTestPrivate.qll new file mode 100644 index 00000000000..ab3a41a4474 --- /dev/null +++ b/ql/test/TestUtilities/InlineExpectationsTestPrivate.qll @@ -0,0 +1,10 @@ +import go + +/** + * Represents a line comment in the Go style. + * include the preceding comment marker (`//`). + */ +class ExpectationComment extends Comment { + /** Returns the contents of the given comment, _without_ the preceding comment marker (`//`). */ + string getContents() { result = this.getText() } +} From 47f40d5f3e1d0e78f4f91bc4ef62817a0791ce58 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 22 Oct 2020 06:42:46 -0700 Subject: [PATCH 15/19] Add tests for log frameworks --- .../concepts/LoggerCall/LoggerCall.expected | 0 .../go/concepts/LoggerCall/LoggerCall.ql | 17 + .../semmle/go/concepts/LoggerCall/glog.go | 53 +++ .../semmle/go/concepts/LoggerCall/go.mod | 9 + .../semmle/go/concepts/LoggerCall/logrus.go | 34 ++ .../semmle/go/concepts/LoggerCall/main.go | 8 + .../semmle/go/concepts/LoggerCall/stdlib.go | 30 ++ .../vendor/github.com/golang/glog/LICENSE | 191 ++++++++++ .../vendor/github.com/golang/glog/stub.go | 50 +++ .../vendor/github.com/sirupsen/logrus/LICENSE | 21 ++ .../vendor/github.com/sirupsen/logrus/stub.go | 327 ++++++++++++++++++ .../LoggerCall/vendor/k8s.io/klog/LICENSE | 191 ++++++++++ .../LoggerCall/vendor/k8s.io/klog/stub.go | 50 +++ .../go/concepts/LoggerCall/vendor/modules.txt | 14 + 14 files changed, 995 insertions(+) create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.expected create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.ql create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/glog.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/go.mod create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/logrus.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/main.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/stdlib.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/LICENSE create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/stub.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/LICENSE create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/stub.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/LICENSE create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/stub.go create mode 100644 ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/modules.txt diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.expected b/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.ql b/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.ql new file mode 100644 index 00000000000..05b18540aed --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/LoggerCall.ql @@ -0,0 +1,17 @@ +import go +import TestUtilities.InlineExpectationsTest + +class LoggerTest extends InlineExpectationsTest { + LoggerTest() { this = "LoggerTest" } + + override string getARelevantTag() { result = "logger" } + + override predicate hasActualResult(string file, int line, string element, string tag, string value) { + exists(LoggerCall log | + log.hasLocationInfo(file, line, _, _, _) and + element = log.toString() and + value = log.getAMessageComponent().toString() and + tag = "logger" + ) + } +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/glog.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/glog.go new file mode 100644 index 00000000000..f73e44fa7bd --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/glog.go @@ -0,0 +1,53 @@ +//go:generate depstubber -vendor github.com/golang/glog "" Error,ErrorDepth,Errorf,Errorln,Exit,ExitDepth,Exitf,Exitln,Fatal,FatalDepth,Fatalf,Fatalln,Info,InfoDepth,Infof,Infoln,Warning,WarningDepth,Warningf,Warningln +//go:generate depstubber -vendor k8s.io/klog "" Error,ErrorDepth,Errorf,Errorln,Exit,ExitDepth,Exitf,Exitln,Fatal,FatalDepth,Fatalf,Fatalln,Info,InfoDepth,Infof,Infoln,Warning,WarningDepth,Warningf,Warningln + +package main + +import ( + "github.com/golang/glog" + "k8s.io/klog" +) + +func glogTest() { + glog.Error(text) // $logger=text + glog.ErrorDepth(0, text) // $f-:logger=text + glog.Errorf(fmt, text) // $logger=fmt $logger=text + glog.Errorln(text) // $logger=text + glog.Exit(text) // $logger=text + glog.ExitDepth(0, text) // $f-:logger=text + glog.Exitf(fmt, text) // $logger=fmt $logger=text + glog.Exitln(text) // $logger=text + glog.Fatal(text) // $logger=text + glog.FatalDepth(0, text) // $f-:logger=text + glog.Fatalf(fmt, text) // $logger=fmt $logger=text + glog.Fatalln(text) // $logger=text + glog.Info(text) // $logger=text + glog.InfoDepth(0, text) // $f-:logger=text + glog.Infof(fmt, text) // $logger=fmt $logger=text + glog.Infoln(text) // $logger=text + glog.Warning(text) // $logger=text + glog.WarningDepth(0, text) // $f-:logger=text + glog.Warningf(fmt, text) // $logger=fmt $logger=text + glog.Warningln(text) // $logger=text + + klog.Error(text) // $logger=text + klog.ErrorDepth(0, text) // $f-:logger=text + klog.Errorf(fmt, text) // $logger=fmt $logger=text + klog.Errorln(text) // $logger=text + klog.Exit(text) // $logger=text + klog.ExitDepth(0, text) // $f-:logger=text + klog.Exitf(fmt, text) // $logger=fmt $logger=text + klog.Exitln(text) // $logger=text + klog.Fatal(text) // $logger=text + klog.FatalDepth(0, text) // $f-:logger=text + klog.Fatalf(fmt, text) // $logger=fmt $logger=text + klog.Fatalln(text) // $logger=text + klog.Info(text) // $logger=text + klog.InfoDepth(0, text) // $f-:logger=text + klog.Infof(fmt, text) // $logger=fmt $logger=text + klog.Infoln(text) // $logger=text + klog.Warning(text) // $logger=text + klog.WarningDepth(0, text) // $f-:logger=text + klog.Warningf(fmt, text) // $logger=fmt $logger=text + klog.Warningln(text) // $logger=text +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/go.mod b/ql/test/library-tests/semmle/go/concepts/LoggerCall/go.mod new file mode 100644 index 00000000000..81d2785a409 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/go.mod @@ -0,0 +1,9 @@ +module codeql-go-tests/concepts/loggercall + +go 1.15 + +require ( + github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b + github.com/sirupsen/logrus v1.7.0 + k8s.io/klog v1.0.0 +) diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/logrus.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/logrus.go new file mode 100644 index 00000000000..c2e65ff6788 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/logrus.go @@ -0,0 +1,34 @@ +//go:generate depstubber -vendor github.com/sirupsen/logrus Fields,LogFunction,Entry WithContext,WithError,WithFields,Error,Fatalf,Panicln,Infof,FatalFn + +package main + +import ( + "context" + "errors" + "github.com/sirupsen/logrus" +) + +func logSomething(entry *logrus.Entry) { + entry.Traceln(text) // $logger=text $f-:logger=fields +} + +func logrusCalls() { + err := errors.New("Error") + 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) // + logSomething(tmp) + + logrus.Error(text) // $logger=text + logrus.Fatalf(fmt, text) // $logger=fmt $logger=text + logrus.Panicln(text) // $logger=text + logrus.Infof(fmt, text) // $logger=fmt $logger=text + logrus.FatalFn(fn) // $logger=fn +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/main.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/main.go new file mode 100644 index 00000000000..bb2111afbec --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/main.go @@ -0,0 +1,8 @@ +package main + +const fmt = "formatted %s string" +const text = "test" + +func main() { + +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/stdlib.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/stdlib.go new file mode 100644 index 00000000000..22e2e5de3d5 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/stdlib.go @@ -0,0 +1,30 @@ +package main + +import ( + "log" +) + +func stdlib() { + var logger log.Logger + logger.SetPrefix("prefix: ") + logger.Fatal(text) // $logger=text + logger.Fatalf(fmt, text) // $logger=fmt $logger=text + logger.Fatalln(text) // $logger=text + logger.Panic(text) // $logger=text + logger.Panicf(fmt, text) // $logger=fmt $logger=text + logger.Panicln(text) // $logger=text + logger.Print(text) // $logger=text + logger.Printf(fmt, text) // $logger=fmt $logger=text + logger.Println(text) // $logger=text + + log.SetPrefix("prefix: ") + log.Fatal(text) // $logger=text + log.Fatalf(fmt, text) // $logger=fmt $logger=text + log.Fatalln(text) // $logger=text + log.Panic(text) // $logger=text + log.Panicf(fmt, text) // $logger=fmt $logger=text + log.Panicln(text) // $logger=text + log.Print(text) // $logger=text + log.Printf(fmt, text) // $logger=fmt $logger=text + log.Println(text) // $logger=text +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/LICENSE b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/LICENSE new file mode 100644 index 00000000000..37ec93a14fd --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/LICENSE @@ -0,0 +1,191 @@ +Apache License +Version 2.0, January 2004 +http://www.apache.org/licenses/ + +TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + +1. Definitions. + +"License" shall mean the terms and conditions for use, reproduction, and +distribution as defined by Sections 1 through 9 of this document. + +"Licensor" shall mean the copyright owner or entity authorized by the copyright +owner that is granting the License. + +"Legal Entity" shall mean the union of the acting entity and all other entities +that control, are controlled by, or are under common control with that entity. +For the purposes of this definition, "control" means (i) the power, direct or +indirect, to cause the direction or management of such entity, whether by +contract or otherwise, or (ii) ownership of fifty percent (50%) or more of the +outstanding shares, or (iii) beneficial ownership of such entity. + +"You" (or "Your") shall mean an individual or Legal Entity exercising +permissions granted by this License. + +"Source" form shall mean the preferred form for making modifications, including +but not limited to software source code, documentation source, and configuration +files. + +"Object" form shall mean any form resulting from mechanical transformation or +translation of a Source form, including but not limited to compiled object code, +generated documentation, and conversions to other media types. + +"Work" shall mean the work of authorship, whether in Source or Object form, made +available under the License, as indicated by a copyright notice that is included +in or attached to the work (an example is provided in the Appendix below). + +"Derivative Works" shall mean any work, whether in Source or Object form, that +is based on (or derived from) the Work and for which the editorial revisions, +annotations, elaborations, or other modifications represent, as a whole, an +original work of authorship. For the purposes of this License, Derivative Works +shall not include works that remain separable from, or merely link (or bind by +name) to the interfaces of, the Work and Derivative Works thereof. + +"Contribution" shall mean any work of authorship, including the original version +of the Work and any modifications or additions to that Work or Derivative Works +thereof, that is intentionally submitted to Licensor for inclusion in the Work +by the copyright owner or by an individual or Legal Entity authorized to submit +on behalf of the copyright owner. For the purposes of this definition, +"submitted" means any form of electronic, verbal, or written communication sent +to the Licensor or its representatives, including but not limited to +communication on electronic mailing lists, source code control systems, and +issue tracking systems that are managed by, or on behalf of, the Licensor for +the purpose of discussing and improving the Work, but excluding communication +that is conspicuously marked or otherwise designated in writing by the copyright +owner as "Not a Contribution." + +"Contributor" shall mean Licensor and any individual or Legal Entity on behalf +of whom a Contribution has been received by Licensor and subsequently +incorporated within the Work. + +2. Grant of Copyright License. + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable copyright license to reproduce, prepare Derivative Works of, +publicly display, publicly perform, sublicense, and distribute the Work and such +Derivative Works in Source or Object form. + +3. Grant of Patent License. + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable (except as stated in this section) patent license to make, have +made, use, offer to sell, sell, import, and otherwise transfer the Work, where +such license applies only to those patent claims licensable by such Contributor +that are necessarily infringed by their Contribution(s) alone or by combination +of their Contribution(s) with the Work to which such Contribution(s) was +submitted. If You institute patent litigation against any entity (including a +cross-claim or counterclaim in a lawsuit) alleging that the Work or a +Contribution incorporated within the Work constitutes direct or contributory +patent infringement, then any patent licenses granted to You under this License +for that Work shall terminate as of the date such litigation is filed. + +4. Redistribution. + +You may reproduce and distribute copies of the Work or Derivative Works thereof +in any medium, with or without modifications, and in Source or Object form, +provided that You meet the following conditions: + +You must give any other recipients of the Work or Derivative Works a copy of +this License; and +You must cause any modified files to carry prominent notices stating that You +changed the files; and +You must retain, in the Source form of any Derivative Works that You distribute, +all copyright, patent, trademark, and attribution notices from the Source form +of the Work, excluding those notices that do not pertain to any part of the +Derivative Works; and +If the Work includes a "NOTICE" text file as part of its distribution, then any +Derivative Works that You distribute must include a readable copy of the +attribution notices contained within such NOTICE file, excluding those notices +that do not pertain to any part of the Derivative Works, in at least one of the +following places: within a NOTICE text file distributed as part of the +Derivative Works; within the Source form or documentation, if provided along +with the Derivative Works; or, within a display generated by the Derivative +Works, if and wherever such third-party notices normally appear. The contents of +the NOTICE file are for informational purposes only and do not modify the +License. You may add Your own attribution notices within Derivative Works that +You distribute, alongside or as an addendum to the NOTICE text from the Work, +provided that such additional attribution notices cannot be construed as +modifying the License. +You may add Your own copyright statement to Your modifications and may provide +additional or different license terms and conditions for use, reproduction, or +distribution of Your modifications, or for any such Derivative Works as a whole, +provided Your use, reproduction, and distribution of the Work otherwise complies +with the conditions stated in this License. + +5. Submission of Contributions. + +Unless You explicitly state otherwise, any Contribution intentionally submitted +for inclusion in the Work by You to the Licensor shall be under the terms and +conditions of this License, without any additional terms or conditions. +Notwithstanding the above, nothing herein shall supersede or modify the terms of +any separate license agreement you may have executed with Licensor regarding +such Contributions. + +6. Trademarks. + +This License does not grant permission to use the trade names, trademarks, +service marks, or product names of the Licensor, except as required for +reasonable and customary use in describing the origin of the Work and +reproducing the content of the NOTICE file. + +7. Disclaimer of Warranty. + +Unless required by applicable law or agreed to in writing, Licensor provides the +Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, +including, without limitation, any warranties or conditions of TITLE, +NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are +solely responsible for determining the appropriateness of using or +redistributing the Work and assume any risks associated with Your exercise of +permissions under this License. + +8. Limitation of Liability. + +In no event and under no legal theory, whether in tort (including negligence), +contract, or otherwise, unless required by applicable law (such as deliberate +and grossly negligent acts) or agreed to in writing, shall any Contributor be +liable to You for damages, including any direct, indirect, special, incidental, +or consequential damages of any character arising as a result of this License or +out of the use or inability to use the Work (including but not limited to +damages for loss of goodwill, work stoppage, computer failure or malfunction, or +any and all other commercial damages or losses), even if such Contributor has +been advised of the possibility of such damages. + +9. Accepting Warranty or Additional Liability. + +While redistributing the Work or Derivative Works thereof, You may choose to +offer, and charge a fee for, acceptance of support, warranty, indemnity, or +other liability obligations and/or rights consistent with this License. However, +in accepting such obligations, You may act only on Your own behalf and on Your +sole responsibility, not on behalf of any other Contributor, and only if You +agree to indemnify, defend, and hold each Contributor harmless for any liability +incurred by, or claims asserted against, such Contributor by reason of your +accepting any such warranty or additional liability. + +END OF TERMS AND CONDITIONS + +APPENDIX: How to apply the Apache License to your work + +To apply the Apache License to your work, attach the following boilerplate +notice, with the fields enclosed by brackets "[]" replaced with your own +identifying information. (Don't include the brackets!) The text should be +enclosed in the appropriate comment syntax for the file format. We also +recommend that a file or class name and description of purpose be included on +the same "printed page" as the copyright notice for easier identification within +third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/stub.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/stub.go new file mode 100644 index 00000000000..7c4ffefc1e8 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/golang/glog/stub.go @@ -0,0 +1,50 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/golang/glog, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/golang/glog (exports: ; functions: Error,ErrorDepth,Errorf,Errorln,Exit,ExitDepth,Exitf,Exitln,Fatal,FatalDepth,Fatalf,Fatalln,Info,InfoDepth,Infof,Infoln,Warning,WarningDepth,Warningf,Warningln) + +// Package glog is a stub of github.com/golang/glog, generated by depstubber. +package glog + +import () + +func Error(_ ...interface{}) {} + +func ErrorDepth(_ int, _ ...interface{}) {} + +func Errorf(_ string, _ ...interface{}) {} + +func Errorln(_ ...interface{}) {} + +func Exit(_ ...interface{}) {} + +func ExitDepth(_ int, _ ...interface{}) {} + +func Exitf(_ string, _ ...interface{}) {} + +func Exitln(_ ...interface{}) {} + +func Fatal(_ ...interface{}) {} + +func FatalDepth(_ int, _ ...interface{}) {} + +func Fatalf(_ string, _ ...interface{}) {} + +func Fatalln(_ ...interface{}) {} + +func Info(_ ...interface{}) {} + +func InfoDepth(_ int, _ ...interface{}) {} + +func Infof(_ string, _ ...interface{}) {} + +func Infoln(_ ...interface{}) {} + +func Warning(_ ...interface{}) {} + +func WarningDepth(_ int, _ ...interface{}) {} + +func Warningf(_ string, _ ...interface{}) {} + +func Warningln(_ ...interface{}) {} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/LICENSE b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/LICENSE new file mode 100644 index 00000000000..f090cb42f37 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Simon Eskildsen + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/stub.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/stub.go new file mode 100644 index 00000000000..edca56aa803 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/github.com/sirupsen/logrus/stub.go @@ -0,0 +1,327 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for github.com/sirupsen/logrus, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: github.com/sirupsen/logrus (exports: Fields,LogFunction,Entry; functions: WithContext,WithError,WithFields,Error,Fatalf,Panicln,Infof,FatalFn) + +// Package logrus is a stub of github.com/sirupsen/logrus, generated by depstubber. +package logrus + +import ( + bytes "bytes" + context "context" + io "io" + runtime "runtime" + time "time" +) + +type Entry struct { + Logger *Logger + Data Fields + Time time.Time + Level Level + Caller *runtime.Frame + Message string + Buffer *bytes.Buffer + Context context.Context +} + +func (_ Entry) HasCaller() bool { + return false +} + +func (_ *Entry) Bytes() ([]byte, error) { + return nil, nil +} + +func (_ *Entry) Debug(_ ...interface{}) {} + +func (_ *Entry) Debugf(_ string, _ ...interface{}) {} + +func (_ *Entry) Debugln(_ ...interface{}) {} + +func (_ *Entry) Error(_ ...interface{}) {} + +func (_ *Entry) Errorf(_ string, _ ...interface{}) {} + +func (_ *Entry) Errorln(_ ...interface{}) {} + +func (_ *Entry) Fatal(_ ...interface{}) {} + +func (_ *Entry) Fatalf(_ string, _ ...interface{}) {} + +func (_ *Entry) Fatalln(_ ...interface{}) {} + +func (_ *Entry) Info(_ ...interface{}) {} + +func (_ *Entry) Infof(_ string, _ ...interface{}) {} + +func (_ *Entry) Infoln(_ ...interface{}) {} + +func (_ *Entry) Log(_ Level, _ ...interface{}) {} + +func (_ *Entry) Logf(_ Level, _ string, _ ...interface{}) {} + +func (_ *Entry) Logln(_ Level, _ ...interface{}) {} + +func (_ *Entry) Panic(_ ...interface{}) {} + +func (_ *Entry) Panicf(_ string, _ ...interface{}) {} + +func (_ *Entry) Panicln(_ ...interface{}) {} + +func (_ *Entry) Print(_ ...interface{}) {} + +func (_ *Entry) Printf(_ string, _ ...interface{}) {} + +func (_ *Entry) Println(_ ...interface{}) {} + +func (_ *Entry) String() (string, error) { + return "", nil +} + +func (_ *Entry) Trace(_ ...interface{}) {} + +func (_ *Entry) Tracef(_ string, _ ...interface{}) {} + +func (_ *Entry) Traceln(_ ...interface{}) {} + +func (_ *Entry) Warn(_ ...interface{}) {} + +func (_ *Entry) Warnf(_ string, _ ...interface{}) {} + +func (_ *Entry) Warning(_ ...interface{}) {} + +func (_ *Entry) Warningf(_ string, _ ...interface{}) {} + +func (_ *Entry) Warningln(_ ...interface{}) {} + +func (_ *Entry) Warnln(_ ...interface{}) {} + +func (_ *Entry) WithContext(_ context.Context) *Entry { + return nil +} + +func (_ *Entry) WithError(_ error) *Entry { + return nil +} + +func (_ *Entry) WithField(_ string, _ interface{}) *Entry { + return nil +} + +func (_ *Entry) WithFields(_ Fields) *Entry { + return nil +} + +func (_ *Entry) WithTime(_ time.Time) *Entry { + return nil +} + +func (_ *Entry) Writer() *io.PipeWriter { + return nil +} + +func (_ *Entry) WriterLevel(_ Level) *io.PipeWriter { + return nil +} + +func Error(_ ...interface{}) {} + +func FatalFn(_ LogFunction) {} + +func Fatalf(_ string, _ ...interface{}) {} + +type Fields map[string]interface{} + +type Formatter interface { + Format(_ *Entry) ([]byte, error) +} + +type Hook interface { + Fire(_ *Entry) error + Levels() []Level +} + +func Infof(_ string, _ ...interface{}) {} + +type Level uint32 + +func (_ Level) MarshalText() ([]byte, error) { + return nil, nil +} + +func (_ Level) String() string { + return "" +} + +func (_ *Level) UnmarshalText(_ []byte) error { + return nil +} + +type LevelHooks map[Level][]Hook + +func (_ LevelHooks) Add(_ Hook) {} + +func (_ LevelHooks) Fire(_ Level, _ *Entry) error { + return nil +} + +type LogFunction func() []interface{} + +type Logger struct { + Out io.Writer + Hooks LevelHooks + Formatter Formatter + ReportCaller bool + Level Level + ExitFunc interface{} +} + +func (_ *Logger) AddHook(_ Hook) {} + +func (_ *Logger) Debug(_ ...interface{}) {} + +func (_ *Logger) DebugFn(_ LogFunction) {} + +func (_ *Logger) Debugf(_ string, _ ...interface{}) {} + +func (_ *Logger) Debugln(_ ...interface{}) {} + +func (_ *Logger) Error(_ ...interface{}) {} + +func (_ *Logger) ErrorFn(_ LogFunction) {} + +func (_ *Logger) Errorf(_ string, _ ...interface{}) {} + +func (_ *Logger) Errorln(_ ...interface{}) {} + +func (_ *Logger) Exit(_ int) {} + +func (_ *Logger) Fatal(_ ...interface{}) {} + +func (_ *Logger) FatalFn(_ LogFunction) {} + +func (_ *Logger) Fatalf(_ string, _ ...interface{}) {} + +func (_ *Logger) Fatalln(_ ...interface{}) {} + +func (_ *Logger) GetLevel() Level { + return 0 +} + +func (_ *Logger) Info(_ ...interface{}) {} + +func (_ *Logger) InfoFn(_ LogFunction) {} + +func (_ *Logger) Infof(_ string, _ ...interface{}) {} + +func (_ *Logger) Infoln(_ ...interface{}) {} + +func (_ *Logger) IsLevelEnabled(_ Level) bool { + return false +} + +func (_ *Logger) Log(_ Level, _ ...interface{}) {} + +func (_ *Logger) LogFn(_ Level, _ LogFunction) {} + +func (_ *Logger) Logf(_ Level, _ string, _ ...interface{}) {} + +func (_ *Logger) Logln(_ Level, _ ...interface{}) {} + +func (_ *Logger) Panic(_ ...interface{}) {} + +func (_ *Logger) PanicFn(_ LogFunction) {} + +func (_ *Logger) Panicf(_ string, _ ...interface{}) {} + +func (_ *Logger) Panicln(_ ...interface{}) {} + +func (_ *Logger) Print(_ ...interface{}) {} + +func (_ *Logger) PrintFn(_ LogFunction) {} + +func (_ *Logger) Printf(_ string, _ ...interface{}) {} + +func (_ *Logger) Println(_ ...interface{}) {} + +func (_ *Logger) ReplaceHooks(_ LevelHooks) LevelHooks { + return nil +} + +func (_ *Logger) SetFormatter(_ Formatter) {} + +func (_ *Logger) SetLevel(_ Level) {} + +func (_ *Logger) SetNoLock() {} + +func (_ *Logger) SetOutput(_ io.Writer) {} + +func (_ *Logger) SetReportCaller(_ bool) {} + +func (_ *Logger) Trace(_ ...interface{}) {} + +func (_ *Logger) TraceFn(_ LogFunction) {} + +func (_ *Logger) Tracef(_ string, _ ...interface{}) {} + +func (_ *Logger) Traceln(_ ...interface{}) {} + +func (_ *Logger) Warn(_ ...interface{}) {} + +func (_ *Logger) WarnFn(_ LogFunction) {} + +func (_ *Logger) Warnf(_ string, _ ...interface{}) {} + +func (_ *Logger) Warning(_ ...interface{}) {} + +func (_ *Logger) WarningFn(_ LogFunction) {} + +func (_ *Logger) Warningf(_ string, _ ...interface{}) {} + +func (_ *Logger) Warningln(_ ...interface{}) {} + +func (_ *Logger) Warnln(_ ...interface{}) {} + +func (_ *Logger) WithContext(_ context.Context) *Entry { + return nil +} + +func (_ *Logger) WithError(_ error) *Entry { + return nil +} + +func (_ *Logger) WithField(_ string, _ interface{}) *Entry { + return nil +} + +func (_ *Logger) WithFields(_ Fields) *Entry { + return nil +} + +func (_ *Logger) WithTime(_ time.Time) *Entry { + return nil +} + +func (_ *Logger) Writer() *io.PipeWriter { + return nil +} + +func (_ *Logger) WriterLevel(_ Level) *io.PipeWriter { + return nil +} + +func Panicln(_ ...interface{}) {} + +func WithContext(_ context.Context) *Entry { + return nil +} + +func WithError(_ error) *Entry { + return nil +} + +func WithFields(_ Fields) *Entry { + return nil +} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/LICENSE b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/LICENSE new file mode 100644 index 00000000000..37ec93a14fd --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/LICENSE @@ -0,0 +1,191 @@ +Apache License +Version 2.0, January 2004 +http://www.apache.org/licenses/ + +TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + +1. Definitions. + +"License" shall mean the terms and conditions for use, reproduction, and +distribution as defined by Sections 1 through 9 of this document. + +"Licensor" shall mean the copyright owner or entity authorized by the copyright +owner that is granting the License. + +"Legal Entity" shall mean the union of the acting entity and all other entities +that control, are controlled by, or are under common control with that entity. +For the purposes of this definition, "control" means (i) the power, direct or +indirect, to cause the direction or management of such entity, whether by +contract or otherwise, or (ii) ownership of fifty percent (50%) or more of the +outstanding shares, or (iii) beneficial ownership of such entity. + +"You" (or "Your") shall mean an individual or Legal Entity exercising +permissions granted by this License. + +"Source" form shall mean the preferred form for making modifications, including +but not limited to software source code, documentation source, and configuration +files. + +"Object" form shall mean any form resulting from mechanical transformation or +translation of a Source form, including but not limited to compiled object code, +generated documentation, and conversions to other media types. + +"Work" shall mean the work of authorship, whether in Source or Object form, made +available under the License, as indicated by a copyright notice that is included +in or attached to the work (an example is provided in the Appendix below). + +"Derivative Works" shall mean any work, whether in Source or Object form, that +is based on (or derived from) the Work and for which the editorial revisions, +annotations, elaborations, or other modifications represent, as a whole, an +original work of authorship. For the purposes of this License, Derivative Works +shall not include works that remain separable from, or merely link (or bind by +name) to the interfaces of, the Work and Derivative Works thereof. + +"Contribution" shall mean any work of authorship, including the original version +of the Work and any modifications or additions to that Work or Derivative Works +thereof, that is intentionally submitted to Licensor for inclusion in the Work +by the copyright owner or by an individual or Legal Entity authorized to submit +on behalf of the copyright owner. For the purposes of this definition, +"submitted" means any form of electronic, verbal, or written communication sent +to the Licensor or its representatives, including but not limited to +communication on electronic mailing lists, source code control systems, and +issue tracking systems that are managed by, or on behalf of, the Licensor for +the purpose of discussing and improving the Work, but excluding communication +that is conspicuously marked or otherwise designated in writing by the copyright +owner as "Not a Contribution." + +"Contributor" shall mean Licensor and any individual or Legal Entity on behalf +of whom a Contribution has been received by Licensor and subsequently +incorporated within the Work. + +2. Grant of Copyright License. + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable copyright license to reproduce, prepare Derivative Works of, +publicly display, publicly perform, sublicense, and distribute the Work and such +Derivative Works in Source or Object form. + +3. Grant of Patent License. + +Subject to the terms and conditions of this License, each Contributor hereby +grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, +irrevocable (except as stated in this section) patent license to make, have +made, use, offer to sell, sell, import, and otherwise transfer the Work, where +such license applies only to those patent claims licensable by such Contributor +that are necessarily infringed by their Contribution(s) alone or by combination +of their Contribution(s) with the Work to which such Contribution(s) was +submitted. If You institute patent litigation against any entity (including a +cross-claim or counterclaim in a lawsuit) alleging that the Work or a +Contribution incorporated within the Work constitutes direct or contributory +patent infringement, then any patent licenses granted to You under this License +for that Work shall terminate as of the date such litigation is filed. + +4. Redistribution. + +You may reproduce and distribute copies of the Work or Derivative Works thereof +in any medium, with or without modifications, and in Source or Object form, +provided that You meet the following conditions: + +You must give any other recipients of the Work or Derivative Works a copy of +this License; and +You must cause any modified files to carry prominent notices stating that You +changed the files; and +You must retain, in the Source form of any Derivative Works that You distribute, +all copyright, patent, trademark, and attribution notices from the Source form +of the Work, excluding those notices that do not pertain to any part of the +Derivative Works; and +If the Work includes a "NOTICE" text file as part of its distribution, then any +Derivative Works that You distribute must include a readable copy of the +attribution notices contained within such NOTICE file, excluding those notices +that do not pertain to any part of the Derivative Works, in at least one of the +following places: within a NOTICE text file distributed as part of the +Derivative Works; within the Source form or documentation, if provided along +with the Derivative Works; or, within a display generated by the Derivative +Works, if and wherever such third-party notices normally appear. The contents of +the NOTICE file are for informational purposes only and do not modify the +License. You may add Your own attribution notices within Derivative Works that +You distribute, alongside or as an addendum to the NOTICE text from the Work, +provided that such additional attribution notices cannot be construed as +modifying the License. +You may add Your own copyright statement to Your modifications and may provide +additional or different license terms and conditions for use, reproduction, or +distribution of Your modifications, or for any such Derivative Works as a whole, +provided Your use, reproduction, and distribution of the Work otherwise complies +with the conditions stated in this License. + +5. Submission of Contributions. + +Unless You explicitly state otherwise, any Contribution intentionally submitted +for inclusion in the Work by You to the Licensor shall be under the terms and +conditions of this License, without any additional terms or conditions. +Notwithstanding the above, nothing herein shall supersede or modify the terms of +any separate license agreement you may have executed with Licensor regarding +such Contributions. + +6. Trademarks. + +This License does not grant permission to use the trade names, trademarks, +service marks, or product names of the Licensor, except as required for +reasonable and customary use in describing the origin of the Work and +reproducing the content of the NOTICE file. + +7. Disclaimer of Warranty. + +Unless required by applicable law or agreed to in writing, Licensor provides the +Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, +including, without limitation, any warranties or conditions of TITLE, +NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are +solely responsible for determining the appropriateness of using or +redistributing the Work and assume any risks associated with Your exercise of +permissions under this License. + +8. Limitation of Liability. + +In no event and under no legal theory, whether in tort (including negligence), +contract, or otherwise, unless required by applicable law (such as deliberate +and grossly negligent acts) or agreed to in writing, shall any Contributor be +liable to You for damages, including any direct, indirect, special, incidental, +or consequential damages of any character arising as a result of this License or +out of the use or inability to use the Work (including but not limited to +damages for loss of goodwill, work stoppage, computer failure or malfunction, or +any and all other commercial damages or losses), even if such Contributor has +been advised of the possibility of such damages. + +9. Accepting Warranty or Additional Liability. + +While redistributing the Work or Derivative Works thereof, You may choose to +offer, and charge a fee for, acceptance of support, warranty, indemnity, or +other liability obligations and/or rights consistent with this License. However, +in accepting such obligations, You may act only on Your own behalf and on Your +sole responsibility, not on behalf of any other Contributor, and only if You +agree to indemnify, defend, and hold each Contributor harmless for any liability +incurred by, or claims asserted against, such Contributor by reason of your +accepting any such warranty or additional liability. + +END OF TERMS AND CONDITIONS + +APPENDIX: How to apply the Apache License to your work + +To apply the Apache License to your work, attach the following boilerplate +notice, with the fields enclosed by brackets "[]" replaced with your own +identifying information. (Don't include the brackets!) The text should be +enclosed in the appropriate comment syntax for the file format. We also +recommend that a file or class name and description of purpose be included on +the same "printed page" as the copyright notice for easier identification within +third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/stub.go b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/stub.go new file mode 100644 index 00000000000..d0357248812 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/k8s.io/klog/stub.go @@ -0,0 +1,50 @@ +// Code generated by depstubber. DO NOT EDIT. +// This is a simple stub for k8s.io/klog, strictly for use in testing. + +// See the LICENSE file for information about the licensing of the original library. +// Source: k8s.io/klog (exports: ; functions: Error,ErrorDepth,Errorf,Errorln,Exit,ExitDepth,Exitf,Exitln,Fatal,FatalDepth,Fatalf,Fatalln,Info,InfoDepth,Infof,Infoln,Warning,WarningDepth,Warningf,Warningln) + +// Package klog is a stub of k8s.io/klog, generated by depstubber. +package klog + +import () + +func Error(_ ...interface{}) {} + +func ErrorDepth(_ int, _ ...interface{}) {} + +func Errorf(_ string, _ ...interface{}) {} + +func Errorln(_ ...interface{}) {} + +func Exit(_ ...interface{}) {} + +func ExitDepth(_ int, _ ...interface{}) {} + +func Exitf(_ string, _ ...interface{}) {} + +func Exitln(_ ...interface{}) {} + +func Fatal(_ ...interface{}) {} + +func FatalDepth(_ int, _ ...interface{}) {} + +func Fatalf(_ string, _ ...interface{}) {} + +func Fatalln(_ ...interface{}) {} + +func Info(_ ...interface{}) {} + +func InfoDepth(_ int, _ ...interface{}) {} + +func Infof(_ string, _ ...interface{}) {} + +func Infoln(_ ...interface{}) {} + +func Warning(_ ...interface{}) {} + +func WarningDepth(_ int, _ ...interface{}) {} + +func Warningf(_ string, _ ...interface{}) {} + +func Warningln(_ ...interface{}) {} diff --git a/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/modules.txt b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/modules.txt new file mode 100644 index 00000000000..c20671b4194 --- /dev/null +++ b/ql/test/library-tests/semmle/go/concepts/LoggerCall/vendor/modules.txt @@ -0,0 +1,14 @@ +# github.com/github/depstubber v0.0.0-20200916130315-f3217697abd4 +## explicit +# github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b +## explicit +github.com/golang/glog +# github.com/sirupsen/logrus v1.7.0 +## explicit +github.com/sirupsen/logrus +# golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 +golang.org/x/sys/unix +golang.org/x/sys/windows +# k8s.io/klog v1.0.0 +## explicit +k8s.io/klog From f0c0a890a52a95aa9db51675bd6117c2f53b7d2a Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 26 Oct 2020 12:25:56 +0000 Subject: [PATCH 16/19] Move OpenUrlRedirect customisation into the query's qll file --- ql/src/semmle/go/frameworks/Revel.qll | 17 ----------------- .../security/OpenUrlRedirectCustomizations.qll | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ql/src/semmle/go/frameworks/Revel.qll b/ql/src/semmle/go/frameworks/Revel.qll index 878d5ff46f8..259513f126f 100644 --- a/ql/src/semmle/go/frameworks/Revel.qll +++ b/ql/src/semmle/go/frameworks/Revel.qll @@ -29,23 +29,6 @@ module Revel { } } - /** - * Reinstate the usual field propagation rules for fields, which the OpenURLRedirect - * query usually excludes, for fields of `Params` other than `Params.Fixed`. - */ - private class PropagateParamsFields extends OpenUrlRedirect::AdditionalStep { - PropagateParamsFields() { this = "PropagateParamsFields" } - - override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(Field f, string field | - f.hasQualifiedName(packagePath(), "Params", field) and - field != "Fixed" - | - succ.(Read).readsField(pred, f) - ) - } - } - private class ParamsBind extends TaintTracking::FunctionModel, Method { ParamsBind() { this.hasQualifiedName(packagePath(), "Params", ["Bind", "BindJSON"]) } diff --git a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll index 1bf433103d9..d33754deb86 100644 --- a/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll +++ b/ql/src/semmle/go/security/OpenUrlRedirectCustomizations.qll @@ -128,3 +128,20 @@ private class UnsafeFieldReadSanitizer extends SafeUrlFlow::SanitizerEdge { ) } } + +/** + * Reinstate the usual field propagation rules for fields, which the OpenURLRedirect + * query usually excludes, for fields of `Params` other than `Params.Fixed`. + */ +private class PropagateParamsFields extends OpenUrlRedirect::AdditionalStep { + PropagateParamsFields() { this = "PropagateParamsFields" } + + override predicate hasTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(Field f, string field | + f.hasQualifiedName(Revel::packagePath(), "Params", field) and + field != "Fixed" + | + succ.(Read).readsField(pred, f) + ) + } +} From 0bf80641e8ad24bc6c0d4701e3698fa50864a129 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 26 Oct 2020 12:26:37 +0000 Subject: [PATCH 17/19] Revel: mark header reads as user-controlled data --- ql/src/semmle/go/frameworks/Revel.qll | 30 +++++++++++++++++++ .../semmle/go/frameworks/Revel/Revel.go | 8 +++++ .../go/frameworks/Revel/TaintFlows.expected | 2 ++ 3 files changed, 40 insertions(+) diff --git a/ql/src/semmle/go/frameworks/Revel.qll b/ql/src/semmle/go/frameworks/Revel.qll index 259513f126f..1bd33768b66 100644 --- a/ql/src/semmle/go/frameworks/Revel.qll +++ b/ql/src/semmle/go/frameworks/Revel.qll @@ -185,4 +185,34 @@ module Revel { override HTTP::ResponseWriter getResponseWriter() { none() } } + + /** + * The getter and setter methods of `revel.RevelHeader`. + * + * Note we currently don't implement `HeaderWrite` and related concepts, as they are currently only used + * to track content-type, and directly setting headers does not seem to be the usual way to set the response + * content-type for this framework. If and when the `HeaderWrite` concept has a more abstract idea of the + * relationship between header-writes and HTTP responses than looking for a particular `http.ResponseWriter` + * instance connecting the two, then we may implement it here for completeness. + */ + private class RevelHeaderMethods extends TaintTracking::FunctionModel { + FunctionInput input; + FunctionOutput output; + string name; + + RevelHeaderMethods() { + this.(Method).hasQualifiedName(packagePath(), "RevelHeader", name) and + ( + name = ["Add", "Set"] and input.isParameter([0, 1]) and output.isReceiver() + or + name = ["Get", "GetAll"] and input.isReceiver() and output.isResult() + or + name = "SetCookie" and input.isParameter(0) and output.isReceiver() + ) + } + + override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) { + inp = input and outp = output + } + } } diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go index ae59e7d1f51..7391b2506f2 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go @@ -126,3 +126,11 @@ func accessingServerRequest(c *revel.Controller) { c.Request.WebSocket.MessageReceiveJSON(&p) // NOT OK usePerson(p) } + +func accessingHeaders(c *revel.Controller) { + tainted := c.Request.Header.Get("somekey") // NOT OK + useString(tainted) + + tainted2 := c.Request.Header.GetAll("somekey") // NOT OK + useString(tainted2[0]) +} diff --git a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected index 51be68608b2..fe8b9115a71 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected +++ b/ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected @@ -39,3 +39,5 @@ | Revel.go:117:12:117:32 | call to UserAgent | Revel.go:117:12:117:32 | call to UserAgent | 117 | | Revel.go:122:37:122:44 | &... : pointer type | Revel.go:123:12:123:18 | message | 122 | | Revel.go:126:41:126:42 | &... : pointer type | Revel.go:127:12:127:12 | p | 126 | +| Revel.go:131:13:131:28 | selection of Header : pointer type | Revel.go:132:12:132:18 | tainted | 131 | +| Revel.go:134:14:134:29 | selection of Header : pointer type | Revel.go:135:12:135:22 | index expression | 134 | From 31cd26fded4946a3bf46d72d69bebe3eaa9d79dd Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 28 Oct 2020 10:12:52 +0100 Subject: [PATCH 18/19] Update links in ql/docs/experimental.md --- ql/docs/experimental.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/docs/experimental.md b/ql/docs/experimental.md index 02a33788b09..2a86f209353 100644 --- a/ql/docs/experimental.md +++ b/ql/docs/experimental.md @@ -17,7 +17,7 @@ Experimental queries and libraries may not be actively maintained as the standar - The query must have a `@name` and `@description` to explain its purpose. - The query must have a `@kind` and `@problem.severity` as required by CodeQL tools. - For details, see the [guide on query metadata](https://github.com/Semmle/ql/blob/master/docs/query-metadata-style-guide.md). + For details, see the [guide on query metadata](https://github.com/github/codeql/blob/master/docs/query-metadata-style-guide.md). 3. **Formatting** @@ -34,4 +34,4 @@ Experimental queries and libraries may not be actively maintained as the standar ## Non-requirements -Other criteria typically required for our standard queries and libraries are not required for experimental queries and libraries. In particular, fully disciplined query [metadata](https://git.semmle.com/Semmle/ql/blob/master/docs/query-metadata-style-guide.md), query [help](https://git.semmle.com/Semmle/ql/blob/master/docs/query-help-style-guide.md), tests, a low false positive rate and performance tuning are not required (but nonetheless recommended). +Other criteria typically required for our standard queries and libraries are not required for experimental queries and libraries. In particular, fully disciplined query [metadata](https://github.com/github/codeql/blob/master/docs/query-metadata-style-guide.md), query [help](https://github.com/github/codeql/blob/master/docs/query-help-style-guide.md), tests, a low false positive rate and performance tuning are not required (but nonetheless recommended). From 1c75c9d1e9bbfd5719fa01eae7554f269718e302 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 28 Oct 2020 14:27:43 +0000 Subject: [PATCH 19/19] Docs: Master -> main and Semmle/ql -> github/codeql everywhere Also fix a reference to QL for Eclipse, and remove some incidental trailing whitespace --- CONTRIBUTING.md | 10 +++++----- ql/docs/experimental.md | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7405af7077e..30a42152aa8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,7 @@ Please note that this project is released with a [Contributor Code of Conduct](C ## Adding a new query -If you have an idea for a query that you would like to share with other CodeQL users, please open a pull request to add it to this repository. +If you have an idea for a query that you would like to share with other CodeQL users, please open a pull request to add it to this repository. Follow the steps below to help other users understand what your query does, and to ensure that your query is consistent with the other CodeQL queries. 1. **Consult the documentation for query writers** @@ -17,14 +17,14 @@ Follow the steps below to help other users understand what your query does, and 2. **Format your code correctly** - All of the standard CodeQL queries and libraries are uniformly formatted for clarity and consistency, so we strongly recommend that all contributions follow the same formatting guidelines. If you use the CodeQL extension for Visual Studio Code, you can auto-format your query in the [QL editor](https://help.semmle.com/ql-for-eclipse/Content/WebHelp/ql-editor.html). For more information, see the [QL style guide](https://github.com/Semmle/ql/blob/master/docs/ql-style-guide.md). + All of the standard CodeQL queries and libraries are uniformly formatted for clarity and consistency, so we strongly recommend that all contributions follow the same formatting guidelines. If you use the CodeQL extension for Visual Studio Code, you can auto-format your query using the [Format Document command](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html). For more information, see the [QL style guide](https://github.com/github/codeql/blob/main/docs/ql-style-guide.md). 3. **Make sure your query has the correct metadata** Query metadata is used to identify your query and make sure the query results are displayed properly. The most important metadata to include are the `@name`, `@description`, and the `@kind`. Other metadata properties (`@precision`, `@severity`, and `@tags`) are usually added after the query has been reviewed by the maintainers. - For more information on writing query metadata, see the [Query metadata style guide](https://github.com/Semmle/ql/blob/master/docs/query-metadata-style-guide.md). + For more information on writing query metadata, see the [Query metadata style guide](https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md). 4. **Make sure the `select` statement is compatible with the query type** @@ -33,8 +33,8 @@ Follow the steps below to help other users understand what your query does, and 5. **Write a query help file** - Query help files explain the purpose of your query to other users. Write your query help in a `.qhelp` file and save it in the same directory as your new query. - For more information on writing query help, see the [Query help style guide](https://github.com/Semmle/ql/blob/master/docs/query-help-style-guide.md). + Query help files explain the purpose of your query to other users. Write your query help in a `.qhelp` file and save it in the same directory as your new query. + For more information on writing query help, see the [Query help style guide](https://github.com/github/codeql/blob/main/docs/query-help-style-guide.md). 6. **Maintain backwards compatibility** diff --git a/ql/docs/experimental.md b/ql/docs/experimental.md index 2a86f209353..1ca9a166b8a 100644 --- a/ql/docs/experimental.md +++ b/ql/docs/experimental.md @@ -17,7 +17,7 @@ Experimental queries and libraries may not be actively maintained as the standar - The query must have a `@name` and `@description` to explain its purpose. - The query must have a `@kind` and `@problem.severity` as required by CodeQL tools. - For details, see the [guide on query metadata](https://github.com/github/codeql/blob/master/docs/query-metadata-style-guide.md). + For details, see the [guide on query metadata](https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md). 3. **Formatting** @@ -34,4 +34,4 @@ Experimental queries and libraries may not be actively maintained as the standar ## Non-requirements -Other criteria typically required for our standard queries and libraries are not required for experimental queries and libraries. In particular, fully disciplined query [metadata](https://github.com/github/codeql/blob/master/docs/query-metadata-style-guide.md), query [help](https://github.com/github/codeql/blob/master/docs/query-help-style-guide.md), tests, a low false positive rate and performance tuning are not required (but nonetheless recommended). +Other criteria typically required for our standard queries and libraries are not required for experimental queries and libraries. In particular, fully disciplined query [metadata](https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md), query [help](https://github.com/github/codeql/blob/main/docs/query-help-style-guide.md), tests, a low false positive rate and performance tuning are not required (but nonetheless recommended).