Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NokogiriSAXEngineTest#test_exception_thrown_on_expansion_attack fails with nokogiri v1.11.0 native gems #41015

Closed
yahonda opened this issue Jan 5, 2021 · 14 comments

Comments

@yahonda
Copy link
Member

yahonda commented Jan 5, 2021

Managed to reproduce NokogiriSAXEngineTest#test_exception_thrown_on_expansion_attack failure since https://buildkite.com/rails/rails/builds/73847#8a8b56c0-ebdf-4a34-8072-62b688412970

Steps to reproduce

git clone https://github.com/rails/rails.git
cd rails
rm Gemfile.lock
bundle install
bundle info nokogiri
cd activesupport
bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

Expected behavior

It should pass.

Actual behavior

It always fails.

$ bundle info nokogiri
  * nokogiri (1.11.0)
	Summary: Nokogiri () makes it easy and painless to work with XML and HTML from Ruby.
	Homepage: https://nokogiri.org
	Documentation: https://nokogiri.org/rdoc/index.html
	Source Code: https://github.com/sparklemotion/nokogiri
	Changelog: https://nokogiri.org/CHANGELOG.html
	Bug Tracker: https://github.com/sparklemotion/nokogiri/issues
	Path: /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.0-x86_64-linux
$ cd activesupport
$ bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"
Run options: --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

# Running:

Fatal error: XML declaration allowed only at the start of the document at :1.
.F

Failure:
NokogiriSAXEngineTest#test_exception_thrown_on_expansion_attack [/home/yahonda/src/github.com/rails/rails/activesupport/test/xml_mini/xml_mini_engine_test.rb:52]:
[RuntimeError] exception expected, not
Class: <NoMethodError>
Message: <"undefined method `<<' for nil:NilClass">
---Backtrace---
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/xml_mini/nokogirisax.rb:62:in `characters'
/home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.0-x86_64-linux/lib/nokogiri/xml/sax/parser.rb:94:in `parse_with'
/home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.0-x86_64-linux/lib/nokogiri/xml/sax/parser.rb:94:in `parse_io'
/home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.0-x86_64-linux/lib/nokogiri/xml/sax/parser.rb:82:in `parse'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/xml_mini/nokogirisax.rb:81:in `parse'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/xml_mini.rb:94:in `parse'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/core_ext/hash/conversions.rb:153:in `initialize'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/core_ext/hash/conversions.rb:130:in `new'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/core_ext/hash/conversions.rb:130:in `from_xml'
/home/yahonda/src/github.com/rails/rails/activesupport/test/xml_mini/xml_mini_engine_test.rb:53:in `block in test_exception_thrown_on_expansion_attack'
---------------


bin/test test/xml_mini/xml_mini_engine_test.rb:51



Finished in 0.202553s, 9.8739 runs/s, 14.8109 assertions/s.
2 runs, 3 assertions, 1 failures, 0 errors, 0 skips
$

System configuration

Rails version:master branch

Ruby version:ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-linux]
It also reproduces with Ruby 2.7.2, 2.6.6 and 2.5.8.

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

It does not reproduce with nokogiri without native gems.

Steps NOT to reproduce

git clone https://github.com/rails/rails.git
cd rails
bundle install # to use rubygems described in Gemfile.lock
bundle info nokogiri
cd activesupport
bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

Result

$ bundle info nokogiri
  * nokogiri (1.10.10)
	Summary: Nokogiri () is an HTML, XML, SAX, and Reader parser
	Homepage: https://nokogiri.org
	Documentation: https://nokogiri.org/rdoc/index.html
	Source Code: https://github.com/sparklemotion/nokogiri
	Changelog: https://nokogiri.org/CHANGELOG.html
	Bug Tracker: https://github.com/sparklemotion/nokogiri/issues
	Path: /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.10.10
$ cd activesupport
$ bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

Run options: --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

# Running:

..

Finished in 0.202464s, 9.8783 runs/s, 14.8174 assertions/s.
2 runs, 3 assertions, 0 failures, 0 errors, 0 skips
$

It may be related to the OS environment because it is a native gem.

$ uname -m
x86_64
$ more /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.10
DISTRIB_CODENAME=groovy
DISTRIB_DESCRIPTION="Ubuntu 20.10"
$

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

Also it does not reproduce with nokogiri v1.11.0 on macOS native gem.

  * nokogiri (1.11.0)
	Summary: Nokogiri () makes it easy and painless to work with XML and HTML from Ruby.
	Homepage: https://nokogiri.org
	Documentation: https://nokogiri.org/rdoc/index.html
	Source Code: https://github.com/sparklemotion/nokogiri
	Changelog: https://nokogiri.org/CHANGELOG.html
	Bug Tracker: https://github.com/sparklemotion/nokogiri/issues
	Path: /Users/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.0-x86_64-darwin
Run options: --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

# Running:

..

Finished in 0.016421s, 121.7953 runs/s, 182.6929 assertions/s.
2 runs, 3 assertions, 0 failures, 0 errors, 0 skips
% bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

Run options: --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

# Running:

..

Finished in 0.016628s, 120.2790 runs/s, 180.4186 assertions/s.
2 runs, 3 assertions, 0 failures, 0 errors, 0 skips
% ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

I think we may need to open an issue at https://github.com/sparklemotion/nokogiri with minimum steps. but have not created it yet.

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

#41017 should workaround this failure.

@eileencodes
Copy link
Member

We can open an issue and point to this at issue / the rails tests. If the gem is going to cause other apps to fail then it'd be best to get that information out earlier rather than later.

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

Thanks for the advice. Sure. Let me open an issue at nokogiri.

@yahonda
Copy link
Member Author

yahonda commented Jan 5, 2021

Opened sparklemotion/nokogiri#2168

@flavorjones
Copy link
Member

Will take a look today, notes will be at sparklemotion/nokogiri#2168.

@flavorjones
Copy link
Member

Yeah, OK, I understand what's going on. Give me a few hours and I'll have a writeup and some PRs.

@flavorjones
Copy link
Member

OK, I've got a PR to Nokogiri running through CI now:

sparklemotion/nokogiri#2169

If it goes green, I'll ship it in v1.11.1 within a few hours.

@flavorjones
Copy link
Member

Nokogiri v1.11.1 has been shipped with a fix.

@yahonda
Copy link
Member Author

yahonda commented Jan 6, 2021

Confirmed nokogiri-1.11.1 address this failure.

$ bundle info nokogiri
  * nokogiri (1.11.1)
	Summary: Nokogiri () makes it easy and painless to work with XML and HTML from Ruby.
	Homepage: https://nokogiri.org
	Documentation: https://nokogiri.org/rdoc/index.html
	Source Code: https://github.com/sparklemotion/nokogiri
	Changelog: https://nokogiri.org/CHANGELOG.html
	Bug Tracker: https://github.com/sparklemotion/nokogiri/issues
	Path: /home/yahonda/.rbenv/versions/3.0.0/lib/ruby/gems/3.0.0/gems/nokogiri-1.11.1-x86_64-linux
yahonda@myryzen:~/src/github.com/rails/rails$ cd activesupport
yahonda@myryzen:~/src/github.com/rails/rails/activesupport$ bin/test test/xml_mini/libxmlsax_engine_test.rb test/xml_mini/nokogirisax_engine_test.rb --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"
Run options: --seed 30319 -n "/^(?:LibXMLSAXEngineTest#(?:test_blank_returns_empty_hash)|NokogiriSAXEngineTest#(?:test_exception_thrown_on_expansion_attack))$/"

# Running:

..

Finished in 0.203437s, 9.8311 runs/s, 14.7466 assertions/s.
2 runs, 3 assertions, 0 failures, 0 errors, 0 skips
$

@yahonda
Copy link
Member Author

yahonda commented Jan 6, 2021

Thanks for the fix. Closing this issue.

@yahonda yahonda closed this as completed Jan 6, 2021
@eileencodes
Copy link
Member

Thanks for the quick fix and detailed investigation @flavorjones!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants