Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit 62c6b0d

Browse files
smowtonsmowton
committed
Add support for more Revel untrusted sources
1 parent 2818da4 commit 62c6b0d

File tree

3 files changed

+55
-38
lines changed

3 files changed

+55
-38
lines changed

‎ql/src/semmle/go/frameworks/Revel.qll‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ module Revel {
7070
exists(string fieldName |
7171
this.getField().hasQualifiedName(packagePath(), "Request", fieldName)
7272
|
73-
fieldName in ["In", "Header", "URL", "Form", "MultipartForm"]
73+
fieldName in ["Header", "ContentType", "AcceptLanguages", "Locale", "URL", "Form",
74+
"MultipartForm"]
7475
)
7576
}
7677
}
@@ -81,38 +82,30 @@ module Revel {
8182
this
8283
.getTarget()
8384
.hasQualifiedName(packagePath(), "Request",
84-
["FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody"])
85+
["FormValue", "PostFormValue", "GetQuery", "GetForm", "GetMultipartForm", "GetBody",
86+
"Cookie", "GetHttpHeader", "GetRequestURI", "MultipartReader", "Referer",
87+
"UserAgent"])
8588
}
8689
}
8790

88-
private class ServerMultipartFormGetFiles extends TaintTracking::FunctionModel, Method {
89-
ServerMultipartFormGetFiles() {
90-
this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetFiles")
91-
}
91+
private class ServerCookieGetValue extends TaintTracking::FunctionModel, Method {
92+
ServerCookieGetValue() { this.hasQualifiedName(packagePath(), "ServerCookie", "GetValue") }
9293

9394
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
9495
inp.isReceiver() and outp.isResult()
9596
}
9697
}
9798

98-
private class ServerMultipartFormGetValues extends TaintTracking::FunctionModel, Method {
99-
ServerMultipartFormGetValues() {
100-
this.hasQualifiedName(packagePath(), "ServerMultipartForm", "GetValues")
99+
private class ServerMultipartFormGetFiles extends TaintTracking::FunctionModel, Method {
100+
ServerMultipartFormGetFiles() {
101+
this.hasQualifiedName(packagePath(), "ServerMultipartForm", ["GetFiles", "GetValues"])
101102
}
102103

103104
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
104105
inp.isReceiver() and outp.isResult()
105106
}
106107
}
107108

108-
private class ServerRequestGet extends TaintTracking::FunctionModel, Method {
109-
ServerRequestGet() { this.hasQualifiedName(packagePath(), "ServerRequest", "Get") }
110-
111-
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
112-
inp.isReceiver() and outp.isResult(0)
113-
}
114-
}
115-
116109
private string contentTypeFromFilename(DataFlow::Node filename) {
117110
if filename.getStringValue().toLowerCase().matches(["%.htm", "%.html"])
118111
then result = "text/html"
@@ -183,8 +176,8 @@ module Revel {
183176
* We extend FileSystemAccess rather than HTTP::ResponseBody as this will usually mean exposing a user-controlled
184177
* file rather than the actual contents being user-controlled.
185178
*/
186-
private class IoUtilFileSystemAccess extends FileSystemAccess::Range, DataFlow::CallNode {
187-
IoUtilFileSystemAccess() {
179+
private class RenderFileNameCall extends FileSystemAccess::Range, DataFlow::CallNode {
180+
RenderFileNameCall() {
188181
this =
189182
any(Method m | m.hasQualifiedName(packagePath(), "Controller", "RenderFileName")).getACall()
190183
}
@@ -195,8 +188,8 @@ module Revel {
195188
/**
196189
* The `revel.Controller.Redirect` method.
197190
*
198-
* For now I assume that in the context `Redirect(url, value)`, where Revel will `Sprintf(url, value)` internally,
199-
* it is very likely `url` imposes some mandatory prefix, so `value` isn't truly an open redirect opportunity.
191+
* It is currently assumed that a tainted `value` in `Redirect(url, value)`, which calls `Sprintf(url, value)`
192+
* internally, cannot lead to an open redirect vulnerability.
200193
*/
201194
private class ControllerRedirectMethod extends HTTP::Redirect::Range, DataFlow::CallNode {
202195
ControllerRedirectMethod() {

‎ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,12 @@ func (c myAppController) accessingParamsJSONIsUnsafe() {
7878
}
7979

8080
func accessingRequestDirectlyIsUnsafe(c *revel.Controller) {
81-
useURLValues(c.Request.GetQuery()) // NOT OK
82-
useURLValues(c.Request.Form) // NOT OK
83-
useURLValues(c.Request.MultipartForm.Value) // NOT OK
81+
useURLValues(c.Request.GetQuery()) // NOT OK
82+
useURLValues(c.Request.Form) // NOT OK
83+
useURLValues(c.Request.MultipartForm.Value) // NOT OK
84+
useString(c.Request.ContentType) // NOT OK
85+
useString(c.Request.AcceptLanguages[0].Language) // NOT OK
86+
useString(c.Request.Locale) // NOT OK
8487

8588
form, _ := c.Request.GetForm() // NOT OK
8689
useURLValues(form)
@@ -95,12 +98,26 @@ func accessingRequestDirectlyIsUnsafe(c *revel.Controller) {
9598

9699
json, _ := ioutil.ReadAll(c.Request.GetBody()) // NOT OK
97100
useJSON(json)
101+
102+
cookie, _ := c.Request.Cookie("abc")
103+
useString(cookie.GetValue()) // NOT OK
104+
105+
useString(c.Request.GetHttpHeader("headername")) // NOT OK
106+
107+
useString(c.Request.GetRequestURI()) // NOT OK
108+
109+
reader, _ := c.Request.MultipartReader()
110+
part, _ := reader.NextPart()
111+
partbody := make([]byte, 100)
112+
part.Read(partbody)
113+
useString(string(partbody)) // NOT OK
114+
115+
useString(c.Request.Referer()) // NOT OK
116+
117+
useString(c.Request.UserAgent()) // NOT OK
98118
}
99119

100120
func accessingServerRequest(c *revel.Controller) {
101-
query, _ := c.Request.In.Get(revel.HTTP_QUERY) // NOT OK
102-
useURLValues(query.(url.Values))
103-
104121
var message string
105122
c.Request.WebSocket.MessageReceive(&message) // NOT OK
106123
useString(message)

‎ql/test/library-tests/semmle/go/frameworks/Revel/TaintFlows.expected‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,21 @@
2121
| Revel.go:82:15:82:28 | selection of Form : Values | Revel.go:33:12:33:23 | call to Get | 82 |
2222
| Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:32:12:32:22 | index expression | 83 |
2323
| Revel.go:83:15:83:37 | selection of MultipartForm : pointer type | Revel.go:33:12:33:23 | call to Get | 83 |
24-
| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:32:12:32:22 | index expression | 85 |
25-
| Revel.go:85:13:85:31 | call to GetForm : tuple type | Revel.go:33:12:33:23 | call to Get | 85 |
26-
| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:32:12:32:22 | index expression | 88 |
27-
| Revel.go:88:13:88:40 | call to GetMultipartForm : tuple type | Revel.go:33:12:33:23 | call to Get | 88 |
28-
| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:92:11:92:35 | index expression | 91 |
29-
| Revel.go:94:11:94:33 | selection of MultipartForm : pointer type | Revel.go:94:11:94:48 | index expression | 94 |
30-
| Revel.go:96:28:96:46 | call to GetBody : Reader | Revel.go:97:10:97:13 | json | 96 |
31-
| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:32:12:32:22 | index expression | 101 |
32-
| Revel.go:101:14:101:25 | selection of In : ServerRequest | Revel.go:33:12:33:23 | call to Get | 101 |
33-
| Revel.go:105:37:105:44 | &... : pointer type | Revel.go:106:12:106:18 | message | 105 |
34-
| Revel.go:109:41:109:42 | &... : pointer type | Revel.go:110:12:110:12 | p | 109 |
24+
| Revel.go:84:12:84:32 | selection of ContentType | Revel.go:84:12:84:32 | selection of ContentType | 84 |
25+
| Revel.go:85:12:85:36 | selection of AcceptLanguages : AcceptLanguages | Revel.go:85:12:85:48 | selection of Language | 85 |
26+
| Revel.go:86:12:86:27 | selection of Locale | Revel.go:86:12:86:27 | selection of Locale | 86 |
27+
| Revel.go:88:13:88:31 | call to GetForm : tuple type | Revel.go:32:12:32:22 | index expression | 88 |
28+
| Revel.go:88:13:88:31 | call to GetForm : tuple type | Revel.go:33:12:33:23 | call to Get | 88 |
29+
| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:32:12:32:22 | index expression | 91 |
30+
| Revel.go:91:13:91:40 | call to GetMultipartForm : tuple type | Revel.go:33:12:33:23 | call to Get | 91 |
31+
| Revel.go:94:13:94:40 | call to GetMultipartForm : tuple type | Revel.go:95:11:95:35 | index expression | 94 |
32+
| Revel.go:97:11:97:33 | selection of MultipartForm : pointer type | Revel.go:97:11:97:48 | index expression | 97 |
33+
| Revel.go:99:28:99:46 | call to GetBody : Reader | Revel.go:100:10:100:13 | json | 99 |
34+
| Revel.go:102:15:102:37 | call to Cookie : tuple type | Revel.go:103:12:103:28 | call to GetValue | 102 |
35+
| Revel.go:105:12:105:48 | call to GetHttpHeader | Revel.go:105:12:105:48 | call to GetHttpHeader | 105 |
36+
| Revel.go:107:12:107:36 | call to GetRequestURI | Revel.go:107:12:107:36 | call to GetRequestURI | 107 |
37+
| Revel.go:109:15:109:41 | call to MultipartReader : tuple type | Revel.go:113:12:113:27 | type conversion | 109 |
38+
| Revel.go:115:12:115:30 | call to Referer | Revel.go:115:12:115:30 | call to Referer | 115 |
39+
| Revel.go:117:12:117:32 | call to UserAgent | Revel.go:117:12:117:32 | call to UserAgent | 117 |
40+
| Revel.go:122:37:122:44 | &... : pointer type | Revel.go:123:12:123:18 | message | 122 |
41+
| Revel.go:126:41:126:42 | &... : pointer type | Revel.go:127:12:127:12 | p | 126 |

0 commit comments

Comments
 (0)