Skip to content

Commit c60cec3

Browse files
committed
add calls to .html_safe? as a shared XSS sanitizer
1 parent db3bf0e commit c60cec3

File tree

3 files changed

+25
-0
lines changed

3 files changed

+25
-0
lines changed

ruby/ql/lib/codeql/ruby/security/XSS.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,19 @@ private module Shared {
248248
or
249249
isFlowFromHelperMethod(node1, node2)
250250
}
251+
252+
private predicate htmlSafeGuard(CfgNodes::AstCfgNode guard, CfgNode testedNode, boolean branch) {
253+
exists(DataFlow::CallNode html_safe_call | html_safe_call.getMethodName() = "html_safe?" |
254+
guard = html_safe_call.asExpr() and
255+
testedNode = html_safe_call.getReceiver().asExpr() and
256+
branch = true
257+
)
258+
}
259+
260+
/** A guard that calls `.html_safe?` to check whether the string is already HTML-safe. */
261+
private class HtmlSafeGuard extends Sanitizer {
262+
HtmlSafeGuard() { this = DataFlow::BarrierGuard<htmlSafeGuard/3>::getABarrierNode() }
263+
}
251264
}
252265

253266
/**

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,19 @@ edges
22
| lib/unsafeHtml.rb:2:31:2:34 | name | lib/unsafeHtml.rb:3:10:3:16 | #{...} | provenance | |
33
| lib/unsafeHtml.rb:9:27:9:30 | name | lib/unsafeHtml.rb:11:13:11:19 | #{...} | provenance | |
44
| lib/unsafeHtml.rb:16:19:16:22 | name | lib/unsafeHtml.rb:17:28:17:31 | name | provenance | |
5+
| lib/unsafeHtml.rb:23:32:23:35 | name | lib/unsafeHtml.rb:24:10:24:16 | #{...} | provenance | |
56
nodes
67
| lib/unsafeHtml.rb:2:31:2:34 | name | semmle.label | name |
78
| lib/unsafeHtml.rb:3:10:3:16 | #{...} | semmle.label | #{...} |
89
| lib/unsafeHtml.rb:9:27:9:30 | name | semmle.label | name |
910
| lib/unsafeHtml.rb:11:13:11:19 | #{...} | semmle.label | #{...} |
1011
| lib/unsafeHtml.rb:16:19:16:22 | name | semmle.label | name |
1112
| lib/unsafeHtml.rb:17:28:17:31 | name | semmle.label | name |
13+
| lib/unsafeHtml.rb:23:32:23:35 | name | semmle.label | name |
14+
| lib/unsafeHtml.rb:24:10:24:16 | #{...} | semmle.label | #{...} |
1215
subpaths
1316
#select
1417
| lib/unsafeHtml.rb:3:10:3:16 | #{...} | lib/unsafeHtml.rb:2:31:2:34 | name | lib/unsafeHtml.rb:3:10:3:16 | #{...} | This string interpolation which depends on $@ might later allow $@. | lib/unsafeHtml.rb:2:31:2:34 | name | library input | lib/unsafeHtml.rb:3:5:3:22 | "<h2>#{...}</h2>" | cross-site scripting |
1518
| lib/unsafeHtml.rb:11:13:11:19 | #{...} | lib/unsafeHtml.rb:9:27:9:30 | name | lib/unsafeHtml.rb:11:13:11:19 | #{...} | This string interpolation which depends on $@ might later allow $@. | lib/unsafeHtml.rb:9:27:9:30 | name | library input | lib/unsafeHtml.rb:13:5:13:5 | h | cross-site scripting |
1619
| lib/unsafeHtml.rb:17:28:17:31 | name | lib/unsafeHtml.rb:16:19:16:22 | name | lib/unsafeHtml.rb:17:28:17:31 | name | This string format which depends on $@ might later allow $@. | lib/unsafeHtml.rb:16:19:16:22 | name | library input | lib/unsafeHtml.rb:17:5:17:32 | call to sprintf | cross-site scripting |
20+
| lib/unsafeHtml.rb:24:10:24:16 | #{...} | lib/unsafeHtml.rb:23:32:23:35 | name | lib/unsafeHtml.rb:24:10:24:16 | #{...} | This string interpolation which depends on $@ might later allow $@. | lib/unsafeHtml.rb:23:32:23:35 | name | library input | lib/unsafeHtml.rb:24:5:24:22 | "<h2>#{...}</h2>" | cross-site scripting |

ruby/ql/test/query-tests/security/cwe-079/lib/unsafeHtml.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,12 @@ def sprintf_use name
1919
# escape
2020
sprintf("<h2>%s</h2>", ERB::Util.html_escape(name)).html_safe # OK - the parameter is escaped
2121
end
22+
23+
def create_user_description2(name)
24+
"<h2>#{name}</h2>".html_safe # NOT OK - the value is not necessarily HTML safe
25+
26+
if name.html_safe?
27+
"<h2>#{name}</h2>".html_safe # OK - value is marked as being HTML safe
28+
end
29+
end
2230
end

0 commit comments

Comments
 (0)