diff --git a/config/initializers/sax_machine.rb b/config/initializers/sax_machine.rb new file mode 100644 index 000000000..33d2106ca --- /dev/null +++ b/config/initializers/sax_machine.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# sax-machine hardcodes `ctx.replace_entities = true` in its Nokogiri handler, +# so a hostile feed can declare an external entity and have us resolve it while +# parsing (XXE): `file://` reads local files and `http://` reaches private +# addresses, both outside the SafeFetch guard that protects the fetch itself. +# +# Turning substitution off only affects entities that point at a SYSTEM +# resource. Predefined (`&`), numeric (`é`) and internally declared +# entities still resolve, so feeds that declare their own HTML entities in a +# DOCTYPE keep working. +module SAXMachine::DisableExternalEntities + def sax_parse(xml_input) + parser = Nokogiri::XML::SAX::Parser.new(self) + parser.parse(xml_input) { |ctx| ctx.replace_entities = false } + end +end + +if SAXMachine.handler == :nokogiri + SAXMachine::SAXNokogiriHandler.prepend(SAXMachine::DisableExternalEntities) +end diff --git a/spec/utils/feedjira_spec.rb b/spec/utils/feedjira_spec.rb new file mode 100644 index 000000000..d58721698 --- /dev/null +++ b/spec/utils/feedjira_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# Guards config/initializers/sax_machine.rb. These assert parser behaviour +# rather than the patch itself, so they still fail if a sax-machine upgrade +# stops the prepend from applying. +RSpec.describe Feedjira do + def secret_file + Tempfile.new("xxe").tap do |file| + file.write("TOP-SECRET-CONTENTS") + file.close + end + end + + def parse_feed(doctype, title) + described_class.parse(<<~XML) + + #{doctype} + + #{title} + http://example.com + + XML + end + + it "does not resolve external entities pointing at local files" do + secret = secret_file + doctype = %(]>) + + expect(parse_feed(doctype, "&xxe;").title.to_s) + .not_to include("TOP-SECRET-CONTENTS") + end + + it "still resolves predefined and numeric entities" do + expect(parse_feed("", "Tom & Jerry café").title) + .to eq("Tom & Jerry cafĂ©") + end + + it "still resolves entities declared inside the document" do + doctype = %(]>) + + expect(parse_feed(doctype, "Feed Title").title) + .to eq("Feed\u00A0Title") + end +end