Skip to content

Commit 1fe6623

Browse files
committed
suggestions based on review: add a popular library example for HTML-sanitization, and use the old text about ../ replacements
1 parent 9db970f commit 1fe6623

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.qhelp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ function removeAllHtmlTags(input) {
8585
return input.replace(/<|>/g, "");
8686
}
8787
</sample>
88+
<p>
89+
Another potential fix is to use the popular <code>sanitize-html</code> npm library.
90+
It keeps most of the safe HTML tags while removing all unsafe tags and attributes.
91+
</p>
92+
<sample language="javascript">
93+
const sanitizeHtml = require("sanitize-html");
94+
function removeAllHtmlTags(input) {
95+
return sanitizeHtml(input);
96+
}
97+
</sample>
8898

8999
</example>
90100

@@ -98,7 +108,10 @@ str.replace(/\.\.\//g, "");
98108
</sample>
99109

100110
<p>
101-
This can result in an unsafe path being generated if only a single replacement is done.
111+
The regular expression attempts to strip out all occurences of <code>/../</code> from <code>str</code>.
112+
This will not work as expected: for the string <code>/./.././</code>, for example, it will remove the single
113+
occurrence of <code>/../</code> in the middle, but the remainder of the string then becomes
114+
<code>/../</code>, which is another instance of the substring we were trying to remove.
102115
</p>
103116

104117
<p>

ruby/ql/src/queries/security/cwe-116/IncompleteMultiCharacterSanitization.qhelp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,18 @@ def remove_all_html_tags(input)
8686
end
8787
</sample>
8888

89+
<p>
90+
Another potential fix is to use the popular <code>sanitize</code> gem.
91+
It keeps most of the safe HTML tags while removing all unsafe tags and attributes.
92+
</p>
93+
<sample language="javascript">
94+
require 'sanitize'
95+
96+
def sanitize_html(input)
97+
Sanitize.fragment(input)
98+
end
99+
</sample>
100+
89101
</example>
90102

91103
<example>
@@ -98,7 +110,10 @@ str.gsub(/\.\.\//, "")
98110
</sample>
99111

100112
<p>
101-
This can result in an unsafe path being generated if only a single replacement is done.
113+
The regular expression attempts to strip out all occurences of <code>/../</code> from <code>str</code>.
114+
This will not work as expected: for the string <code>/./.././</code>, for example, it will remove the single
115+
occurrence of <code>/../</code> in the middle, but the remainder of the string then becomes
116+
<code>/../</code>, which is another instance of the substring we were trying to remove.
102117
</p>
103118

104119
<p>

0 commit comments

Comments
 (0)