OpenUrlRedirect: only make some parts of the URL untrusted

This commit is contained in:
Sauyon Lee
2020-01-08 23:48:52 -08:00
parent 2b92cd5ba5
commit 82635a46ad
2 changed files with 44 additions and 1 deletions

View File

@@ -30,8 +30,39 @@ module OpenUrlRedirect {
/**
* A source of third-party user input, considered as a flow source for URL redirects.
*
* Excludes the URL field, as it is handled more precisely in `UntrustedUrlField`.
*/
class UntrustedFlowAsSource extends Source, UntrustedFlowSource { }
class UntrustedFlowAsSource extends Source, UntrustedFlowSource {
UntrustedFlowAsSource() {
forex(SelectorExpr sel | this.asExpr() = sel | sel.getSelector().getName() != "URL")
}
}
/**
* An access to a user-controlled URL field, considered as a flow source for URL redirects.
*/
class UntrustedUrlField extends Source, DataFlow::ExprNode {
override SelectorExpr expr;
UntrustedUrlField() {
exists(Type req, Type baseType, string fieldName |
req.hasQualifiedName("net/http", "Request") and
baseType = expr.getBase().(SelectorExpr).getBase().getType() and
expr.getBase().(SelectorExpr).getSelector().getName() = "URL" and
(baseType = req or baseType = req.getPointerType()) and
fieldName = expr.getSelector().getName() and
(
fieldName = "User" or
fieldName = "Path" or
fieldName = "RawPath" or
fieldName = "ForceQuery" or
fieldName = "RawQuery" or
fieldName = "Fragment"
)
)
}
}
/**
* An HTTP redirect, considered as a sink for `Configuration`.

View File

@@ -103,5 +103,17 @@ func serveStdlib() {
}
})
http.HandleFunc("/ex8", func(w http.ResponseWriter, r *http.Request) {
r.ParseForm()
// GOOD: this only rewrites the scheme, which is not dangerous as the host cannot change.
if r.URL.Scheme == "http" {
r.URL.Scheme = "https"
http.Redirect(w, r, r.URL.String(), 302)
} else {
// ...
}
})
http.ListenAndServe(":80", nil)
}