rails/rails-html-sanitizer

test failures when using loofah 2.13.0 and nokogiri 1.12.5

Segaja opened this issue ยท 7 comments

When trying to package the latest version for Archlinux i get these test errors:

/usr/bin/ruby -w -I"lib" /usr/lib/ruby/gems/3.0.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb"
Run options: --seed 10816

# Running:

..................F....................................................................................................................................F...F...................................................................................F........................................................................

Finished in 0.101214s, 3082.5645 runs/s, 3260.4048 assertions/s.

  1) Failure:
SanitizersTest#test_uri_escaping_of_href_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:491]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a href=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a href=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  2) Failure:
SanitizersTest#test_uri_escaping_of_name_action_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:521]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a action=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a action=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  3) Failure:
SanitizersTest#test_uri_escaping_of_src_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:501]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a src=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a src=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  4) Failure:
SanitizersTest#test_uri_escaping_of_name_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:511]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a name=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a name=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


312 runs, 330 assertions, 4 failures, 0 errors, 0 skips

I would appreciate a fix for these ;)

I cannot reproduce what you're describing.

juno in rails-html-sanitizer on ๎‚  HEAD (c86fed1) [!?] via ๐Ÿ’Ž v3.0.3 
$ ruby -v
ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]

juno in rails-html-sanitizer on ๎‚  HEAD (c86fed1) [!?] via ๐Ÿ’Ž v3.0.3 
$ bundle show
Gems included by the bundle:
  * activesupport (7.0.1)
  * bundler (2.3.3)
  * concurrent-ruby (1.1.9)
  * crass (1.0.6)
  * i18n (1.8.11)
  * loofah (2.13.0)
  * minitest (5.15.0)
  * nokogiri (1.12.5)
  * racc (1.6.0)
  * rails-dom-testing (2.0.3)
  * rails-html-sanitizer (1.4.2)
  * rake (13.0.6)
  * tzinfo (2.0.4)

juno in rails-html-sanitizer on ๎‚  HEAD (c86fed1) [!?] via ๐Ÿ’Ž v3.0.3 
$ be rake test
/home/flavorjones/.rbenv/versions/3.0.3/bin/ruby -w -I"lib" /home/flavorjones/.rbenv/versions/3.0.3/lib/ruby/gems/3.0.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb" 
Run options: --seed 9295

# Running:

........................................................................................................................................................................................................................................................................................................................

Finished in 0.187622s, 1662.9184 runs/s, 1758.8560 assertions/s.

312 runs, 330 assertions, 0 failures, 0 errors, 0 skips

Can you please provide the output of nokogiri -v?

OK, I think what you're seeing is Nokogiri compiled against the system libxml2 which is likely based on 2.9.10 and hasn't applied the patch for this vulnerability yet: https://bugzilla.gnome.org/show_bug.cgi?id=769760 which is fixed by https://gitlab.gnome.org/GNOME/libxml2/-/commit/c1ba6f5 reverting GNOME/libxml2@960f0e2

Nokogiri has patched this in its vendored libxml2 since v1.8.3.

My advice is to ask the libxml2 maintainer for Arch to pull in that patch; or else skip those tests. It's unfortunately not easy to determine if the system library has specific patches applied or not.

@flavorjones I'm sorry to say I can't confirm your asumption. Archlinux is using libxml 2.9.12 ( https://archlinux.org/packages/extra/x86_64/libxml2/ ). I tried to rebuild the packages in the dependency chain between libxml2 and ruby-rails-html-sanitizer but the error persist:

Here are some debug information:

ruby -v:

ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]

bundle show:

Gems included by the bundle:
  * activesupport (6.1.4.1)
  * bundler (2.2.26)
  * concurrent-ruby (1.1.8)
  * crass (1.0.6)
  * i18n (1.8.11)
  * loofah (2.13.0)
  * mini_portile2 (2.7.1)
  * minitest (5.15.0)
  * nokogiri (1.13.1)
  * racc (1.5.2)
  * rails-dom-testing (2.0.3)
  * rails-html-sanitizer (1.4.2)
  * rake (13.0.6)
  * tzinfo (2.0.4)
  * zeitwerk (2.5.3)

nokogiri -v:

# Nokogiri (1.13.1)
    ---
    warnings: []
    nokogiri:
      version: 1.13.1
      cppflags:
      - "-I/usr/lib/ruby/gems/3.0.0/gems/nokogiri-1.13.1/ext/nokogiri"
      ldflags: []
    ruby:
      version: 3.0.3
      platform: x86_64-linux
      gem_platform: x86_64-linux
      description: ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]
      engine: ruby
    libxml:
      source: system
      memory_management: ruby
      iconv_enabled: true
      compiled: 2.9.12
      loaded: 2.9.12
    libxslt:
      source: system
      datetime_enabled: true
      compiled: 1.1.34
      loaded: 1.1.34
    other_libraries:
      libgumbo: 1.0.0-nokogiri

rake test:

/usr/bin/ruby -w -I"lib" /usr/lib/ruby/gems/3.0.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb"
Run options: --seed 3346

# Running:

................................................................................................................F..........................................................................F...............................F.................F..........................................................................

Finished in 0.097523s, 3199.2539 runs/s, 3383.8262 assertions/s.

  1) Failure:
SanitizersTest#test_uri_escaping_of_href_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:491]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a href=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a href=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  2) Failure:
SanitizersTest#test_uri_escaping_of_src_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:501]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a src=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a src=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  3) Failure:
SanitizersTest#test_uri_escaping_of_name_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:511]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a name=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a name=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


  4) Failure:
SanitizersTest#test_uri_escaping_of_name_action_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.4.2/test/sanitizer_test.rb:521]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a action=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a action=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


312 runs, 330 assertions, 4 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1): [ruby -w -I"lib" /usr/lib/ruby/gems/3.0.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb" ]

Tasks: TOP => test
(See full trace by running task with --trace)

Any further help on this subject would be appreciated.

Best regards
Segaja

#126 updates the tests on master

I can confirm that #126 fixes the issues. Also I still don't believe that arch runs with an unpatched libxml2. For now I will add these changes as patch to the build.

Is there any plan to release a new version any time soon to get this change into a release?

I am not planning to make a public release simply because tests were updated.

Also, to be clear, the patch in question has not been accepted upstream, it's something Nokogiri chooses to patch around.