From 392f1f5a18698874fbfb367e3cea443dd7eace09 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 25 Jan 2018 01:52:34 +0100 Subject: [PATCH] Fix relayable signatures for messages with invalid XML characters Sometimes messages contain characters that are invalid for XML, but they are filteres out before creating the XML, otherwise the property would be empty in the XML. But for relayables the value is also used for creating the signatures, so we need to filter the invalid characters earlier, before calculating the signature. --- lib/diaspora_federation/entity.rb | 4 ++-- .../entities/relayable_spec.rb | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index 30549b32..7b564bef 100644 --- a/lib/diaspora_federation/entity.rb +++ b/lib/diaspora_federation/entity.rb @@ -277,7 +277,7 @@ def normalize_property(name, value) case self.class.class_props[name] when :string - value.to_s + value.to_s.gsub(INVALID_XML_REGEX, "\uFFFD") when :timestamp value.nil? ? "" : value.utc.iso8601 else @@ -310,7 +310,7 @@ def add_property_to_xml(doc, root_element, name, value) # Create simple node, fill it with text and append to root def simple_node(doc, name, value) Nokogiri::XML::Element.new(name.to_s, doc).tap do |node| - node.content = value.gsub(INVALID_XML_REGEX, "\uFFFD") unless value.empty? + node.content = value unless value.empty? end end diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index 10471418..f80e143f 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb @@ -208,6 +208,22 @@ module DiasporaFederation expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy end + it "computes correct signatures for the entity with invalid XML characters" do + expect_callback(:fetch_private_key, author).and_return(author_pkey) + expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey) + + invalid_property = "asdfasdf asdf💩asdf\nasdf" + signature_data_with_fixed_property = "#{author};#{guid};#{parent_guid};asdf�asdf asdf💩asdf\nasdf" + + xml = Entities::SomeRelayable.new(hash.merge(property: invalid_property)).to_xml + + author_signature = xml.at_xpath("author_signature").text + parent_author_signature = xml.at_xpath("parent_author_signature").text + + expect(verify_signature(author_pkey, author_signature, signature_data_with_fixed_property)).to be_truthy + expect(verify_signature(parent_pkey, parent_author_signature, signature_data_with_fixed_property)).to be_truthy + end + it "computes correct signatures for the entity when the parent is a relayable itself" do intermediate_author = Fabricate.sequence(:diaspora_id) parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: local_parent)