Pilot #0: Add web framework `clevergo` #438

gagliardetto · 2020-12-20T22:44:52Z

This is a "pilot" batch (of 1 element) for a project targeting web frameworks.

Let me know your thoughts.

gagliardetto · 2020-12-20T22:56:24Z

Right now the tool can handle the UntrustedFlowSource and TaintTracking models.

Given that I don't want to open more than one PR (ever) for any given web framework, I want to be sure I get the most important models right from the start. It's like going to the grocery store; I want to get everything the first time I go there.

This is the list of models I want to add:

HTTP::Redirect::Range
HTTP::ResponseBody::Range

Those two seem like the most impactful. Anything else?

gagliardetto · 2020-12-20T23:06:08Z

Maybe also:

HTTP::Redirect::UnexploitableSource
TaintTracking::DefaultTaintSanitizer

I'm aiming at having the most impact, rather than completeness. The most bang for the buck.

smowton · 2020-12-21T13:25:23Z

Most of the Go team is out for the holidays now, so I'll look on the 4th January or later

gagliardetto · 2020-12-21T14:09:26Z

Sounds great! Thanks! Happy holidays

smowton · 2020-12-21T14:11:05Z

You too :)

ql/src/semmle/go/frameworks/CleverGo.qll

sauyon

Thanks again, I should really try to get the tool working so I can use it myself.

I haven't actually looked at the models themselves, but a few comments on the generator.

ql/src/semmle/go/frameworks/CleverGo.qll

ql/test/library-tests/semmle/go/frameworks/CleverGo/TaintTracking/Test.ql

ql/test/library-tests/semmle/go/frameworks/CleverGo/UntrustedSources/Test.ql

gagliardetto · 2020-12-22T23:45:04Z

Thanks again, I should really try to get the tool working so I can use it myself.

Thanks for the review! This is actually not codebox but another tool I recently wrote from (almost) scratch. There are a few things I want to do with it before I opensource it.

smowton

You can see some of the other common ::Range classes used in the newly-added Beego models (https://github.com/github/codeql-go/blob/main/ql/src/semmle/go/frameworks/Beego.qll)

Particularly HeaderWrite and ResponseWriter, which are used to pair up a response body with its content-type. Note that there are methods that set both simultaneously (e.g. response.JSON(xyz)), those that explicitly set content-type (response.SetContentType("text/html"); response.Write(xyz)) and those that set a generic HTTP header which could be Content-Type (response.SetHeader("Content-Type", "text/html"); response.Write(xyz))

ql/src/semmle/go/frameworks/CleverGo.qll

gagliardetto · 2021-01-24T17:12:52Z

Next I'm going to add HTTP::Redirect.

I'm thinking it could look like this:

  private class RedirectMethods2 extends HTTP::Redirect::Range, DataFlow::CallNode {
    string package;
    DataFlow::Node urlNode;

    RedirectMethods2() {
      package = packagePath() and
      (
        // Function call:
        this.getTarget().hasQualifiedName(package, "DoRedirect") and
        urlNode = this.getArgument(2)
        or
        // Method on type call:
        this = any(Method m | m.hasQualifiedName(package, "SomeReceiver", "Redirect")).getACall() and
        urlNode = this.getArgument(1)
        or
        // Method on interface call:
        this = any(Method m | m.implements(package, "SomeInterface", "Redirect")).getACall() and
        urlNode = this.getArgument(1)
      )
    }

    override DataFlow::Node getUrl() { result = urlNode }

    //    override HTTP::ResponseWriter getResponseWriter() { result.getANode() = this.getReceiver() }
    override HTTP::ResponseWriter getResponseWriter() { none() }
  }

Questions:

What is getResponseWriter() exactly for?
Is having getResponseWriter() impactful?

Thanks!

smowton · 2021-01-25T11:37:17Z

For header writes, getResponseWriter is for pairing up a header-set operation with the corresponding response body, which the XSS query uses to exclude bodies accompanied by a non-HTML content-type. For HTTP::Redirect it doesn't appear to be used at the moment, so can be safely set to none() and perhaps ought to be removed.

gagliardetto · 2021-01-25T13:08:42Z

Thanks @smowton

And how does the proposed model look to you?

smowton · 2021-01-25T15:21:14Z

Looks good to me -- @sauyon you have any further comments?

gagliardetto · 2021-01-25T16:27:17Z

Added the modelling for HTTP redirects.

Not sure about the correctness of the inline tests query; does https://github.com/github/codeql-go/blob/9d6ac53acff98b33aacc1c7824670518e83eb4f7/ql/test/library-tests/semmle/go/frameworks/CleverGo/HttpRedirect/Test.ql actually catch what it is supposed to catch? Do they make sense?

gagliardetto · 2021-01-25T17:39:56Z

Once the HTTP::Redirect model is approved, I'll move on to adding another kind of model (haven't decided which yet).

Once a number of model kinds have been implemented for this web framework, I'll tidy up this PR to be merged (things like removing duplicate vendoring, etc.).

gagliardetto · 2021-01-26T17:00:09Z

Next up: HTTP::ResponseBody

  private class ResponseBody extends HTTP::ResponseBody::Range {
    string package;
    DataFlow::CallNode call;
    string contentType;

    ResponseBody() {
      package = packagePath() and
      (
        // Function call:
        call.getTarget().hasQualifiedName(package, "JSON") and
        this = call.getArgument(1) and
        contentType = "application/json"
        or
        // Method on type call:
        call = any(Method m | m.hasQualifiedName(package, "SomeReceiver", "JSON")).getACall() and
        this = call.getArgument(0) and
        contentType = "application/json"
        or
        // Method on interface call:
        call = any(Method m | m.implements(package, "SomeInterface", "JSON")).getACall() and
        this = call.getArgument(0) and
        contentType = "application/json"
      )
    }

    override string getAContentType() { result = contentType }

    override HTTP::ResponseWriter getResponseWriter() { none() }
  }

Questions:

What is getResponseWriter() exactly for?
Is having getResponseWriter() impactful?

sauyon · 2021-01-27T06:01:45Z

1. What is `getResponseWriter()` exactly for?

For linking response writes with other response writes and headers. If the model has another way of achieving this, omitting it is fine.

sauyon · 2021-01-27T06:02:42Z

Not sure about the correctness of the inline tests query; does https://github.com/github/codeql-go/blob/9d6ac53acff98b33aacc1c7824670518e83eb4f7/ql/test/library-tests/semmle/go/frameworks/CleverGo/HttpRedirect/Test.ql actually catch what it is supposed to catch? Do they make sense?

The inline test query makes sense to me; was there a problem with it?

gagliardetto · 2021-01-27T14:48:27Z

Not sure about the correctness of the inline tests query; does https://github.com/github/codeql-go/blob/9d6ac53acff98b33aacc1c7824670518e83eb4f7/ql/test/library-tests/semmle/go/frameworks/CleverGo/HttpRedirect/Test.ql actually catch what it is supposed to catch? Do they make sense?

The inline test query makes sense to me; was there a problem with it?

Nope. Just wanted to makes sure it made sense.

gagliardetto · 2021-01-27T16:37:49Z

Added HTTP::ResponseBody modelling.

Next will think about HTTP::ResponseWriter. Still not sure about how it works and relates to the getResponseWriter() methods.

gagliardetto · 2021-02-03T18:09:27Z

Got a few more scenarios of ResponseBody without using HTTP::ResponseWriter (I hope that's OK).

gagliardetto · 2021-02-09T16:25:04Z

I added more HTTP::ResponseBody models and tests which consider all possible combinations.

TODO:

To find whether methods are called on the same receiver, I use: contentTypeSetterCall.getReceiver().getAPredecessor*() = bodySetterCall.getReceiver().getAPredecessor*(); is that correct and a good way to do it?
How would you implement override HTTP::ResponseWriter getResponseWriter()?
~~Add tests for the case where the contentType is set via a HeaderWrite.~~ DONE
~~Remove duplicate vendoring.~~ DONE

gagliardetto · 2021-02-09T17:21:04Z

One thing to note is that the Content-Type header value might look something like text/html; charset=UTF-8, and to match that you'd need some regex.

A second thing to note is that this code snippet from the tests does not work (using a variable for the contentType string):

			// func (*Context).Blob(code int, contentType string, bs []byte) (err error)
			{
				bodyByte839 := source().([]byte)
				contentTypeString458 := "application/json"
				var rece clevergo.Context
				rece.Blob(0, contentTypeString458, bodyByte839) // $contentType=application/json $responseBody=bodyByte839
			}

I had to change it to this (the content-type is a string passed directly as parameter in the call):

			// func (*Context).Blob(code int, contentType string, bs []byte) (err error)
			{
				bodyByte839 := source().([]byte)
				var rece clevergo.Context
				rece.Blob(0, "application/json", bodyByte839) // $contentType=application/json $responseBody=bodyByte839
			}

Maybe there's something wrong in the codeql models?

…ted folders for each test.

smowton · 2021-04-09T07:40:22Z

Rebased and merging on green

gagliardetto · 2021-04-09T09:00:09Z

That's cool! But can you please help me solve the questions above?

gagliardetto · 2021-04-09T10:37:09Z

More precisely, there are a couple questions and quirks I want to make sure get solved before I proceed further: #438 (comment) #438 (comment)

gagliardetto · 2021-04-10T12:55:39Z

For future reference, here is the codemill spec file for clevergo:

CleverGo.json:

{
    "Name": "CleverGo",
    "Models": [
    {
        "Name": "UntrustedSources",
        "Kind": "UntrustedFlowSource",
        "Methods": [
        {
            "Name": "Self",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-BasicAuth",
                    "Pos": [false, true, true, false],
                    "Flows": null,
                    "Name": "BasicAuth"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Decode",
                    "Pos": [false, true, false],
                    "Flows": null,
                    "Name": "Decode"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-DefaultQuery",
                    "Pos": [false, false, false, true],
                    "Flows": null,
                    "Name": "DefaultQuery"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-FormValue",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "FormValue"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-GetHeader",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "GetHeader"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-PostFormValue",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "PostFormValue"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-QueryParam",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "QueryParam"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-QueryString",
                    "Pos": [false, true],
                    "Flows": null,
                    "Name": "QueryString"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Params-String",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "String"
                }
            },
            {
                "Kind": "Struct",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "Struct-Context",
                    "TypeName": "Context",
                    "Fields":
                    {
                        "Params": null
                    }
                }
            },
            {
                "Kind": "Struct",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "Struct-Param",
                    "TypeName": "Param",
                    "Fields":
                    {
                        "Key": null,
                        "Value": null
                    }
                }
            },
            {
                "Kind": "Type",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "Type-Params",
                    "TypeName": "Params",
                    "Value": true
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "InterfaceMethod-Decoder-Decode",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "Decode"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-QueryParams",
                    "Pos": [false, true],
                    "Flows": null,
                    "Name": "QueryParams"
                }
            }]
        }]
    },
    {
        "Name": "TaintTracking",
        "Kind": "TaintTracking",
        "Methods": [
        {
            "Name": "Self",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "Function-CleanPath",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [true, false],
                            "Out": [false, true]
                        }],
                        "Enabled": true
                    },
                    "Name": "CleanPath"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "InterfaceMethod-Decoder-Decode",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [false, true, false, false],
                            "Out": [false, false, true, false]
                        }],
                        "Enabled": true
                    },
                    "Name": "Decode"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "InterfaceMethod-Renderer-Render",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [false, false, false, true, false, false],
                            "Out": [false, true, false, false, false, false]
                        }],
                        "Enabled": true
                    },
                    "Name": "Render"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Application-RouteURL",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [false, true, true, false, false],
                            "Out": [false, false, false, true, false]
                        }],
                        "Enabled": true
                    },
                    "Name": "RouteURL"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Context",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [true, false],
                            "Out": [false, true]
                        }],
                        "Enabled": true
                    },
                    "Name": "Context"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Params-String",
                    "Pos": null,
                    "Flows":
                    {
                        "Blocks": [
                        {
                            "Inp": [true, false, false],
                            "Out": [false, false, true]
                        }],
                        "Enabled": true
                    },
                    "Name": "String"
                }
            }]
        }]
    },
    {
        "Name": "HttpRedirect",
        "Kind": "HTTP::Redirect",
        "Methods": [
        {
            "Name": "GetUrl",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Redirect",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "Redirect"
                }
            }]
        }]
    },
    {
        "Name": "HttpResponseBody",
        "Kind": "HTTP::ResponseBody",
        "Methods": [
        {
            "Name": "(body+ctFromFuncName):body",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Error",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "Error"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-HTML",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "HTML"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-HTMLBlob",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "HTMLBlob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSON",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "JSON"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSONBlob",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "JSONBlob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSONP",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "JSONP"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSONPBlob",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "JSONPBlob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSONPCallback",
                    "Pos": [false, false, false, true, false],
                    "Flows": null,
                    "Name": "JSONPCallback"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-JSONPCallbackBlob",
                    "Pos": [false, false, false, true, false],
                    "Flows": null,
                    "Name": "JSONPCallbackBlob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-String",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "String"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-StringBlob",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "StringBlob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Stringf",
                    "Pos": [false, false, true, true, false],
                    "Flows": null,
                    "Name": "Stringf"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-XML",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "XML"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-XMLBlob",
                    "Pos": [false, false, true, false],
                    "Flows": null,
                    "Name": "XMLBlob"
                }
            }]
        },
        {
            "Name": "(body+ct):body",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Blob",
                    "Pos": [false, false, false, true, false],
                    "Flows": null,
                    "Name": "Blob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Emit",
                    "Pos": [false, false, false, true, false],
                    "Flows": null,
                    "Name": "Emit"
                }
            }]
        },
        {
            "Name": "(body+ct):ct",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Blob",
                    "Pos": [false, false, true, false, false],
                    "Flows": null,
                    "Name": "Blob"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Emit",
                    "Pos": [false, false, true, false, false],
                    "Flows": null,
                    "Name": "Emit"
                }
            }]
        },
        {
            "Name": ":body",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-Write",
                    "Pos": [false, true, false, false],
                    "Flows": null,
                    "Name": "Write"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-WriteString",
                    "Pos": [false, true, false, false],
                    "Flows": null,
                    "Name": "WriteString"
                }
            }]
        },
        {
            "Name": ":ct",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetContentType",
                    "Pos": [false, true],
                    "Flows": null,
                    "Name": "SetContentType"
                }
            }]
        },
        {
            "Name": ":ctFromFuncName",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetContentTypeHTML",
                    "Pos": [true],
                    "Flows": null,
                    "Name": "SetContentTypeHTML"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetContentTypeJSON",
                    "Pos": [true],
                    "Flows": null,
                    "Name": "SetContentTypeJSON"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetContentTypeText",
                    "Pos": [true],
                    "Flows": null,
                    "Name": "SetContentTypeText"
                }
            },
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetContentTypeXML",
                    "Pos": [true],
                    "Flows": null,
                    "Name": "SetContentTypeXML"
                }
            }]
        }]
    },
    {
        "Name": "HeaderWrite",
        "Kind": "HTTP::HeaderWrite",
        "Methods": [
        {
            "Name": "WriteKey",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetHeader",
                    "Pos": [false, true, false],
                    "Flows": null,
                    "Name": "SetHeader"
                }
            }]
        },
        {
            "Name": "WriteVal",
            "Selectors": [
            {
                "Kind": "Func",
                "Qualifier":
                {
                    "Path": "clevergo.tech/clevergo",
                    "Version": "v0.5.2",
                    "ID": "TypeMethod-Context-SetHeader",
                    "Pos": [false, false, true],
                    "Flows": null,
                    "Name": "SetHeader"
                }
            }]
        }]
    }]
}

sauyon · 2021-04-12T08:25:56Z

One thing to note is that the Content-Type header value might look something like text/html; charset=UTF-8, and to match that you'd need some regex.

We just use .*html.*.

codeql-go/ql/src/semmle/go/security/Xss.qll

Lines 89 to 91 in dbcf1e1

    
           private predicate htmlTypeSpecified(HTTP::ResponseBody body) { 
        
             body.getAContentType().regexpMatch("(?i).*html.*") 
        
           }

To find whether methods are called on the same receiver, I use: contentTypeSetterCall.getReceiver().getAPredecessor*() = bodySetterCall.getReceiver().getAPredecessor*(); is that correct and a good way to do it?

That's fine. Elsewhere we use VariableWithFields as well as .getAPredecessor*, but I'm not sure it really adds much.

How would you implement override HTTP::ResponseWriter getResponseWriter()?

As this is only currently used in a not-very-nice way to link response writes and header writes together, possibly not at all. Perhaps a better interface here could be something like .getAHeaderWrite and / or .getAnotherWrite or something along those lines.

Add tests for the case where the contentType is set via a HeaderWrite.

No comments here.

Remove duplicate vendoring.

At some point I'd like to do this, but it's very much not a priority.

gagliardetto · 2021-04-12T12:33:00Z

One thing to note is that the Content-Type header value might look something like text/html; charset=UTF-8, and to match that you'd need some regex.

We just use .*html.*.

Makes sense!

To find whether methods are called on the same receiver, I use: contentTypeSetterCall.getReceiver().getAPredecessor*() = bodySetterCall.getReceiver().getAPredecessor*(); is that correct and a good way to do it?

That's fine. Elsewhere we use VariableWithFields as well as .getAPredecessor*, but I'm not sure it really adds much.

👍

How would you implement override HTTP::ResponseWriter getResponseWriter()?

As this is only currently used in a not-very-nice way to link response writes and header writes together, possibly not at all. Perhaps a better interface here could be something like .getAHeaderWrite and / or .getAnotherWrite or something along those lines.

I'll continue to leave override HTTP::ResponseWriter getResponseWriter() out of my models.

Add tests for the case where the contentType is set via a HeaderWrite.

No comments here.

Forgot to cross that out. (Done)

Remove duplicate vendoring.

At some point I'd like to do this, but it's very much not a priority.

Forgot to cross that out. (Done) (Was referring to the vendor duplication in clevergo tests)

Thanks @sauyon !

airairair20 suggested changes Dec 21, 2020

View reviewed changes

ql/src/semmle/go/frameworks/CleverGo.qll Outdated Show resolved Hide resolved

sauyon reviewed Dec 22, 2020

View reviewed changes

smowton reviewed Jan 4, 2021

View reviewed changes

ql/src/semmle/go/frameworks/CleverGo.qll Outdated Show resolved Hide resolved

ql/src/semmle/go/frameworks/CleverGo.qll Outdated Show resolved Hide resolved

intrigus-lgtm reviewed Jan 5, 2021

View reviewed changes

ql/src/semmle/go/frameworks/CleverGo.qll Outdated Show resolved Hide resolved

gagliardetto force-pushed the clevergo branch from 7d31650 to 10a3bf1 Compare January 23, 2021 09:46

This comment has been minimized.

Sign in to view

gagliardetto marked this pull request as draft February 8, 2021 20:30

gagliardetto marked this pull request as ready for review February 9, 2021 16:26

gagliardetto requested review from smowton and sauyon February 9, 2021 16:27

gagliardetto added 18 commits April 9, 2021 08:38

Regenerate tests

54abdf1

Simplify UntrustedSources interface methods

c01259e

Simplify UntrustedSources struct fields

d3d7d2d

Cleanup comments

bdc5f90

Cleanup comments

f95f353

Use //go:generate depstubber --vendor --auto

0c1ae62

Regenerate dep stubs

1de7196

Make naming more consistent

55c8d9b

Add http redirect model

c53d8d3

Fix autoformatting

98b3cc2

Add models for HTTP::ResponseBody

0fe7050

Use docs instead of comments for classes.

93ff245

Model HTTP::HeaderWrite; regenerate stubs

7edf739

Simplify HTTP::HeaderWrite

8c18aa6

Refactor and improve HTTP:ResponseBody models and tests

3915305

Move clevergo framework to experimental

7ea0434

Improve naming of files and elements.

4ae5bdb

Put all tests file in to the CleverGo folder instead of having dedica…

8e839f3

…ted folders for each test.

smowton force-pushed the clevergo branch 2 times, most recently from 4282bd4 to 8e839f3 Compare April 9, 2021 07:39

smowton enabled auto-merge April 9, 2021 07:40

smowton approved these changes Apr 9, 2021

View reviewed changes

smowton added the no-change-note-required label Apr 9, 2021

smowton merged commit 46b5f11 into github:main Apr 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pilot #0: Add web framework `clevergo` #438

Pilot #0: Add web framework `clevergo` #438

gagliardetto commented Dec 20, 2020 •

edited

Loading

gagliardetto commented Dec 20, 2020

gagliardetto commented Dec 20, 2020

smowton commented Dec 21, 2020

gagliardetto commented Dec 21, 2020

smowton commented Dec 21, 2020

sauyon left a comment

gagliardetto commented Dec 22, 2020

smowton left a comment

gagliardetto commented Jan 24, 2021

smowton commented Jan 25, 2021

gagliardetto commented Jan 25, 2021

smowton commented Jan 25, 2021

gagliardetto commented Jan 25, 2021 •

edited

Loading

gagliardetto commented Jan 25, 2021 •

edited

Loading

gagliardetto commented Jan 26, 2021

sauyon commented Jan 27, 2021

sauyon commented Jan 27, 2021

gagliardetto commented Jan 27, 2021

gagliardetto commented Jan 27, 2021

gagliardetto commented Feb 3, 2021

This comment has been minimized.

gagliardetto commented Feb 9, 2021 •

edited

Loading

gagliardetto commented Feb 9, 2021 •

edited

Loading

smowton commented Apr 9, 2021

gagliardetto commented Apr 9, 2021 via email •

edited

Loading

gagliardetto commented Apr 9, 2021 via email

gagliardetto commented Apr 10, 2021

sauyon commented Apr 12, 2021

gagliardetto commented Apr 12, 2021

Pilot #0: Add web framework clevergo #438

Pilot #0: Add web framework clevergo #438

Conversation

gagliardetto commented Dec 20, 2020 • edited Loading

gagliardetto commented Dec 20, 2020

gagliardetto commented Dec 20, 2020

smowton commented Dec 21, 2020

gagliardetto commented Dec 21, 2020

smowton commented Dec 21, 2020

sauyon left a comment

Choose a reason for hiding this comment

gagliardetto commented Dec 22, 2020

smowton left a comment

Choose a reason for hiding this comment

gagliardetto commented Jan 24, 2021

smowton commented Jan 25, 2021

gagliardetto commented Jan 25, 2021

smowton commented Jan 25, 2021

gagliardetto commented Jan 25, 2021 • edited Loading

gagliardetto commented Jan 25, 2021 • edited Loading

gagliardetto commented Jan 26, 2021

sauyon commented Jan 27, 2021

sauyon commented Jan 27, 2021

gagliardetto commented Jan 27, 2021

gagliardetto commented Jan 27, 2021

gagliardetto commented Feb 3, 2021

This comment has been minimized.

gagliardetto commented Feb 9, 2021 • edited Loading

TODO:

gagliardetto commented Feb 9, 2021 • edited Loading

smowton commented Apr 9, 2021

gagliardetto commented Apr 9, 2021 via email • edited Loading

gagliardetto commented Apr 9, 2021 via email

gagliardetto commented Apr 10, 2021

sauyon commented Apr 12, 2021

gagliardetto commented Apr 12, 2021

Pilot #0: Add web framework `clevergo` #438

Pilot #0: Add web framework `clevergo` #438

gagliardetto commented Dec 20, 2020 •

edited

Loading

gagliardetto commented Jan 25, 2021 •

edited

Loading

gagliardetto commented Jan 25, 2021 •

edited

Loading

gagliardetto commented Feb 9, 2021 •

edited

Loading

gagliardetto commented Feb 9, 2021 •

edited

Loading

gagliardetto commented Apr 9, 2021 via email •

edited

Loading