Python: Add HTTP::Server::CookieWrite concept

along with tests, but no implementations (to ease reviewing).

---

I've put quite some thinking into what to call our concept for this.

[JS has `CookieDefinition`](581f4ed757/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll (L148-L187)), but I couldn't find a matching concept in any other languages.

We used to call this [`CookieSet`](f07a7bf8cf/python/ql/src/semmle/python/web/Http.qll (L76)) (and had a corresponding `CookieGet`).

But for headers, [Go calls this `HeaderWrite`](cd1e14ed09/ql/src/semmle/go/concepts/HTTP.qll (L97-L131)) and [JS calls this `HeaderDefinition`](581f4ed757/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll (L23-L46))

I think it would be really cool if we have a naming scheme that means the name for getting the value of a header on a incoming request is obvious. I think `HeaderWrite`/`HeaderRead` fulfils this best. We could go with `HeaderSet`/`HeaderGet`, but they feel a bit too vague to me. For me, I'm so used to talking about def-use, that I would immediately go for `HeaderDefinition` and `HeaderUse`, which could work, but is kinda strange.

So in the end that means I went with `CookieWrite`, since that allows using a consistent naming scheme for the future :)
This commit is contained in:
Rasmus Wriedt Larsen
2021-06-24 14:34:28 +02:00
parent 902b450b12
commit e1af1f11ee
7 changed files with 151 additions and 6 deletions

View File

@@ -555,6 +555,62 @@ module HTTP {
abstract DataFlow::Node getRedirectLocation();
}
}
/**
* A data-flow node that sets a cookie in an HTTP response.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `HTTP::CookieWrite::Range` instead.
*/
class CookieWrite extends DataFlow::Node {
CookieWrite::Range range;
CookieWrite() { this = range }
/**
* Gets the argument, if any, specifying the raw cookie header.
*/
DataFlow::Node getHeaderArg() { result = range.getHeaderArg() }
/**
* Gets the argument, if any, specifying the cookie name.
*/
DataFlow::Node getNameArg() { result = range.getNameArg() }
/**
* Gets the argument, if any, specifying the cookie value.
*/
DataFlow::Node getValueArg() { result = range.getValueArg() }
}
/** Provides a class for modeling new cookie writes on HTTP responses. */
module CookieWrite {
/**
* A data-flow node that sets a cookie in an HTTP response.
*
* Note: we don't require that this redirect must be sent to a client (a kind of
* "if a tree falls in a forest and nobody hears it" situation).
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `HttpResponse` instead.
*/
abstract class Range extends DataFlow::Node {
/**
* Gets the argument, if any, specifying the raw cookie header.
*/
abstract DataFlow::Node getHeaderArg();
/**
* Gets the argument, if any, specifying the cookie name.
*/
abstract DataFlow::Node getNameArg();
/**
* Gets the argument, if any, specifying the cookie value.
*/
abstract DataFlow::Node getValueArg();
}
}
}
}

View File

@@ -252,6 +252,38 @@ class HttpServerHttpRedirectResponseTest extends InlineExpectationsTest {
}
}
class HttpServerCookieWriteTest extends InlineExpectationsTest {
HttpServerCookieWriteTest() { this = "HttpServerCookieWriteTest" }
override string getARelevantTag() {
result in ["CookieWrite", "CookieRawHeader", "CookieName", "CookieValue"]
}
override predicate hasActualResult(Location location, string element, string tag, string value) {
exists(location.getFile().getRelativePath()) and
exists(HTTP::Server::CookieWrite cookieWrite |
location = cookieWrite.getLocation() and
(
element = cookieWrite.toString() and
value = "" and
tag = "CookieWrite"
or
element = cookieWrite.toString() and
value = prettyNodeForInlineTest(cookieWrite.getHeaderArg()) and
tag = "CookieRawHeader"
or
element = cookieWrite.toString() and
value = prettyNodeForInlineTest(cookieWrite.getNameArg()) and
tag = "CookieName"
or
element = cookieWrite.toString() and
value = prettyNodeForInlineTest(cookieWrite.getValueArg()) and
tag = "CookieValue"
)
)
}
}
class FileSystemAccessTest extends InlineExpectationsTest {
FileSystemAccessTest() { this = "FileSystemAccessTest" }

View File

@@ -65,6 +65,20 @@ async def redirect_302(request): # $ requestHandler
else:
raise web.HTTPFound(location="/logout") # $ HttpResponse HttpRedirectResponse mimetype=application/octet-stream redirectLocation="/logout"
################################################################################
# Cookies
################################################################################
@routes.get("/setting_cookie") # $ routeSetup="/setting_cookie"
async def setting_cookie(request): # $ requestHandler
resp = web.Response(text="foo") # $ HttpResponse mimetype=text/plain responseBody="foo"
resp.cookies["key"] = "value" # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
resp.set_cookie("key3", "value3") # $ MISSING: CookieWrite CookieName="key3" CookieValue="value3"
resp.set_cookie(name="key3", value="value3") # $ MISSING: CookieWrite CookieName="key3" CookieValue="value3"
resp.del_cookie("key4") # $ MISSING: CookieWrite CookieName="key4"
return resp
if __name__ == "__main__":
app = web.Application()

View File

@@ -103,3 +103,15 @@ class CustomJsonResponse(JsonResponse):
def safe__custom_json_response(request):
return CustomJsonResponse("ACME Responses", {"foo": request.GET.get("foo")}) # $HttpResponse mimetype=application/json MISSING: responseBody=Dict SPURIOUS: responseBody="ACME Responses"
################################################################################
# Cookies
################################################################################
def setting_cookie(request):
resp = HttpResponse() # $ HttpResponse mimetype=text/html
resp.set_cookie("key", "value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key", value="value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
resp.cookies["key3"] = "value3" # $ MISSING: CookieWrite CookieName="key3" CookieValue="value3"
return resp

View File

@@ -184,6 +184,19 @@ def redirect_simple(): # $requestHandler
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
################################################################################
# Cookies
################################################################################
@app.route("/setting_cookie") # $routeSetup="/setting_cookie"
def setting_cookie(): # $requestHandler
resp = make_response() # $ HttpResponse mimetype=text/html
resp.set_cookie("key", "value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key", value="value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
resp.headers.add("Set-Cookie", "key2=value2") # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
resp.delete_cookie("key3") # $ MISSING: CookieWrite CookieName="key3"
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
################################################################################

View File

@@ -58,6 +58,18 @@ class ExampleConnectionWrite(tornado.web.RequestHandler):
stream.write(b"foo stream") # $ MISSING: HttpResponse responseBody=b"foo stream"
stream.close()
################################################################################
# Cookies
################################################################################
class CookieWriting(tornado.web.RequestHandler):
def get(self): # $ requestHandler
self.write("foo") # $ HttpResponse mimetype=text/html responseBody="foo"
self.set_cookie("key", "value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
self.set_cookie(name="key", value="value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
self.set_header("Set-Cookie", "key2=value2") # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
def make_app():
return tornado.web.Application(
[
@@ -66,6 +78,7 @@ def make_app():
(r"/ExampleRedirect", ExampleRedirect), # $ routeSetup="/ExampleRedirect"
(r"/ExampleConnectionWrite", ExampleConnectionWrite), # $ routeSetup="/ExampleConnectionWrite"
(r"/ExampleConnectionWrite/(stream)", ExampleConnectionWrite), # $ routeSetup="/ExampleConnectionWrite/(stream)"
(r"/CookieWriting", CookieWriting), # $ routeSetup="/CookieWriting"
],
debug=True,
)
@@ -74,6 +87,7 @@ def make_app():
if __name__ == "__main__":
import tornado.ioloop
print("running on http://localhost:8888/")
app = make_app()
app.listen(8888)
tornado.ioloop.IOLoop.current().start()

View File

@@ -43,16 +43,20 @@ class Redirect(Resource):
# requested with curl.
return b"hello" # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=b"hello"
################################################################################
# Cookies
################################################################################
class NonHttpBodyOutput(Resource):
class CookieWriting(Resource):
"""Examples of providing values in response that is not in the body
"""
def render_GET(self, request: Request): # $ requestHandler
request.responseHeaders.addRawHeader("key", "value")
request.setHeader("key2", "value")
request.addCookie("key", "value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
request.addCookie(k="key", v="value") # $ MISSING: CookieWrite CookieName="key" CookieValue="value"
request.cookies.append("key2=value") # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
request.addCookie("key", "value")
request.cookies.append(b"key2=value")
request.responseHeaders.addRawHeader("key", "value")
request.setHeader("Set-Cookie", "key3=value3") # $ MISSING: CookieWrite CookieRawHeader="key3=value3"
return b"" # $ HttpResponse mimetype=text/html responseBody=b""
@@ -62,7 +66,7 @@ root.putChild(b"also-now", AlsoNow())
root.putChild(b"later", Later())
root.putChild(b"plain-text", PlainText())
root.putChild(b"redirect", Redirect())
root.putChild(b"non-body", NonHttpBodyOutput())
root.putChild(b"setting_cookie", CookieWriting())
if __name__ == "__main__":