Skip to content

Commit d4919d0

Browse files
committed
add a taint-step for format-calls
1 parent f222cc1 commit d4919d0

File tree

3 files changed

+25
-0
lines changed

3 files changed

+25
-0
lines changed

ruby/ql/lib/codeql/ruby/frameworks/StringFormatters.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,19 @@ private class StringLiteralFormatStep extends AdditionalTaintStep {
9393
pred.asExpr() = succ.asExpr().(CfgNodes::ExprNodes::StringlikeLiteralCfgNode).getAComponent()
9494
}
9595
}
96+
97+
/**
98+
* A taint propagating data flow edge arising from string formatting.
99+
*/
100+
private class StringFormattingTaintStep extends AdditionalTaintStep {
101+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
102+
exists(PrintfStyleCall call |
103+
call.returnsFormatted() and
104+
succ = call
105+
|
106+
pred = call.getFormatString()
107+
or
108+
pred = call.getFormatArgument(_)
109+
)
110+
}
111+
}

ruby/ql/test/query-tests/security/cwe-079/StoredXSS.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ edges
1111
| app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text |
1212
| app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] |
1313
| app/views/foo/stores/show.html.erb:41:76:41:87 | call to display_text : | app/views/foo/stores/show.html.erb:41:64:41:87 | ... + ... : |
14+
| app/views/foo/stores/show.html.erb:87:17:87:28 | call to handle : | app/views/foo/stores/show.html.erb:87:3:87:29 | call to sprintf |
1415
nodes
1516
| app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | semmle.label | call to read : |
1617
| app/controllers/foo/stores_controller.rb:9:22:9:23 | dt : | semmle.label | dt : |
@@ -31,6 +32,8 @@ nodes
3132
| app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | semmle.label | call to raw_name |
3233
| app/views/foo/stores/show.html.erb:80:5:80:22 | call to display_name | semmle.label | call to display_name |
3334
| app/views/foo/stores/show.html.erb:83:5:83:24 | @other_user_raw_name | semmle.label | @other_user_raw_name |
35+
| app/views/foo/stores/show.html.erb:87:3:87:29 | call to sprintf | semmle.label | call to sprintf |
36+
| app/views/foo/stores/show.html.erb:87:17:87:28 | call to handle : | semmle.label | call to handle : |
3437
subpaths
3538
#select
3639
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | Stored cross-site scripting vulnerability due to $@. | app/controllers/foo/stores_controller.rb:8:10:8:29 | call to read | stored value |
@@ -46,3 +49,4 @@ subpaths
4649
| app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | Stored cross-site scripting vulnerability due to $@. | app/views/foo/stores/show.html.erb:70:3:70:20 | call to raw_name | stored value |
4750
| app/views/foo/stores/show.html.erb:80:5:80:22 | call to display_name | app/views/foo/stores/show.html.erb:80:5:80:22 | call to display_name | app/views/foo/stores/show.html.erb:80:5:80:22 | call to display_name | Stored cross-site scripting vulnerability due to $@. | app/views/foo/stores/show.html.erb:80:5:80:22 | call to display_name | stored value |
4851
| app/views/foo/stores/show.html.erb:83:5:83:24 | @other_user_raw_name | app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name : | app/views/foo/stores/show.html.erb:83:5:83:24 | @other_user_raw_name | Stored cross-site scripting vulnerability due to $@. | app/controllers/foo/stores_controller.rb:12:28:12:48 | call to raw_name | stored value |
52+
| app/views/foo/stores/show.html.erb:87:3:87:29 | call to sprintf | app/views/foo/stores/show.html.erb:87:17:87:28 | call to handle : | app/views/foo/stores/show.html.erb:87:3:87:29 | call to sprintf | Stored cross-site scripting vulnerability due to $@. | app/views/foo/stores/show.html.erb:87:17:87:28 | call to handle | stored value |

ruby/ql/test/query-tests/security/cwe-079/app/views/foo/stores/show.html.erb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,8 @@
8181

8282
<%# BAD: Indirect to a database value without escaping %>
8383
<%= @other_user_raw_name.html_safe %>
84+
85+
<%# BAD: Kernel.sprintf is a taint-step %>
86+
<%=
87+
sprintf("%s", @user.handle).html_safe
88+
%>

0 commit comments

Comments
 (0)