Skip to content

Commit a11498d

Browse files
committed
Fix sanitization bypass in HTML foreign content
GHSA-p4x4-rw2p-8j8m
1 parent 1d0d688 commit a11498d

File tree

4 files changed

+45
-11
lines changed

4 files changed

+45
-11
lines changed

Diff for: README.md

+11
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ Sanitize can sanitize the following types of input:
7272
* Standalone CSS stylesheets
7373
* Standalone CSS properties
7474

75+
However, please note that Sanitize _cannot_ fully sanitize the contents of
76+
`<math>` or `<svg>` elements, since these elements don't follow the same parsing
77+
rules as the rest of HTML. If this is something you need, you may want to look
78+
for another solution.
79+
7580
### HTML Fragments
7681

7782
A fragment is a snippet of HTML that doesn't contain a root-level `<html>`
@@ -415,6 +420,12 @@ elements not in this array will be removed.
415420
]
416421
```
417422

423+
**Warning:** Sanitize cannot fully sanitize the contents of `<math>` or `<svg>`
424+
elements, since these elements don't follow the same parsing rules as the rest
425+
of HTML. If you add `math` or `svg` to the allowlist, you must assume that any
426+
content inside them will be allowed, even if that content would otherwise be
427+
removed by Sanitize.
428+
418429
#### :parser_options (Hash)
419430

420431
[Parsing options](https://github.com/rubys/nokogumbo/tree/v2.0.1#parsing-options) supplied to `nokogumbo`.

Diff for: lib/sanitize/config/default.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ module Config
7474
# the specified elements (when filtered) will be removed, and the contents
7575
# of all other filtered elements will be left behind.
7676
:remove_contents => %w[
77-
iframe noembed noframes noscript script style
77+
iframe math noembed noframes noscript plaintext script style svg xmp
7878
],
7979

8080
# Transformers allow you to filter or alter nodes using custom logic. See

Diff for: test/test_clean_element.rb

+20-10
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,16 @@
192192
.must_equal ''
193193
end
194194

195-
it 'should escape the content of removed `plaintext` elements' do
196-
Sanitize.fragment('<plaintext>hello! <script>alert(0)</script>')
197-
.must_equal 'hello! &lt;script&gt;alert(0)&lt;/script&gt;'
198-
end
199-
200-
it 'should escape the content of removed `xmp` elements' do
201-
Sanitize.fragment('<xmp>hello! <script>alert(0)</script></xmp>')
202-
.must_equal 'hello! &lt;script&gt;alert(0)&lt;/script&gt;'
203-
end
204-
205195
it 'should not preserve the content of removed `iframe` elements' do
206196
Sanitize.fragment('<iframe>hello! <script>alert(0)</script></iframe>')
207197
.must_equal ''
208198
end
209199

200+
it 'should not preserve the content of removed `math` elements' do
201+
Sanitize.fragment('<math>hello! <script>alert(0)</script></math>')
202+
.must_equal ''
203+
end
204+
210205
it 'should not preserve the content of removed `noembed` elements' do
211206
Sanitize.fragment('<noembed>hello! <script>alert(0)</script></noembed>')
212207
.must_equal ''
@@ -222,6 +217,11 @@
222217
.must_equal ''
223218
end
224219

220+
it 'should not preserve the content of removed `plaintext` elements' do
221+
Sanitize.fragment('<plaintext>hello! <script>alert(0)</script>')
222+
.must_equal ''
223+
end
224+
225225
it 'should not preserve the content of removed `script` elements' do
226226
Sanitize.fragment('<script>hello! <script>alert(0)</script></script>')
227227
.must_equal ''
@@ -232,6 +232,16 @@
232232
.must_equal ''
233233
end
234234

235+
it 'should not preserve the content of removed `svg` elements' do
236+
Sanitize.fragment('<svg>hello! <script>alert(0)</script></svg>')
237+
.must_equal ''
238+
end
239+
240+
it 'should not preserve the content of removed `xmp` elements' do
241+
Sanitize.fragment('<xmp>hello! <script>alert(0)</script></xmp>')
242+
.must_equal ''
243+
end
244+
235245
strings.each do |name, data|
236246
it "should clean #{name} HTML" do
237247
Sanitize.fragment(data[:html]).must_equal(data[:default])

Diff for: test/test_malicious_html.rb

+13
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,17 @@
219219
end
220220
end
221221
end
222+
223+
# https://github.com/rgrove/sanitize/security/advisories/GHSA-p4x4-rw2p-8j8m
224+
describe 'foreign content bypass in relaxed config' do
225+
it 'prevents a sanitization bypass via carefully crafted foreign content' do
226+
%w[iframe noembed noframes noscript plaintext script style xmp].each do |tag_name|
227+
@s.fragment(%[<math><#{tag_name}>/*&lt;/#{tag_name}&gt;&lt;img src onerror=alert(1)>*/]).
228+
must_equal ''
229+
230+
@s.fragment(%[<svg><#{tag_name}>/*&lt;/#{tag_name}&gt;&lt;img src onerror=alert(1)>*/]).
231+
must_equal ''
232+
end
233+
end
234+
end
222235
end

0 commit comments

Comments
 (0)