Fix XSS via OpenGraph on mobile

Changelog.md

@@ -1,3 +1,7 @@
+# 0.7.7.1
+
+Fixes a potential cross-site scripting issue with maliciously crafted OpenGraph metadata on the mobile interface.
+
 # 0.7.7.0
 
 ## Refactor

app/helpers/open_graph_helper.rb

@@ -1,16 +1,6 @@
 # frozen_string_literal: true
 
 module OpenGraphHelper
-  def og_html(cache)
-    "<a href=\"#{cache.url}\" target=\"_blank\">" +
-    "  <div>" +
-    "    <img src=\"#{cache.image}\" />" +
-    "    <strong>#{cache.title}</strong>" +
-    "    <p>#{truncate(cache.description, length: 250, separator: ' ')}</p>" +
-    "  </div>" +
-    "</a>"
-  end
-
   def link_to_oembed_image(cache, prefix = 'thumbnail_')
     link_to(oembed_image_tag(cache, prefix), cache.url, :target => '_blank')
   end

app/views/status_messages/_status_message.mobile.haml

@@ -20,4 +20,10 @@
     != o_embed_html post.o_embed_cache
   - if post.open_graph_cache
     .opengraph
-      != og_html post.open_graph_cache
+      %a{href: post.open_graph_cache.url, target: "_blank"}
+        %div
+          = image_tag post.open_graph_cache.image
+          %strong
+            = post.open_graph_cache.title
+          %p
+            = truncate(post.open_graph_cache.description, length: 250, separator: " ")

spec/helpers/open_graph_helper_spec.rb

@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-describe OpenGraphHelper, :type => :helper do
-
-  describe 'og_html' do
-    scenarios = {
-      "article" => {
-        "url" => "http://opengraph-enabled-site.com/articles/1332-scientists-discover-new-planet",
-        "image" => "http://opengraph-enabled-site.com/images/1332-lead.jpg",
-        "title" => "Scientists discover new planet",
-        "description" => "A new planet was found yesterday"
-      },
-    }
-
-    scenarios.each do |type, data|
-      specify 'for type "'+type+'"' do
-        cache =  OpenGraphCache.new(:url => data['url'])
-        cache.ob_type = type
-        cache.image = data['image']
-        cache.title = data['title']
-        cache.description = data['description']
-
-        formatted = og_html(cache)
-
-        expect(formatted).to match(/#{data['url']}/)
-        expect(formatted).to match(/#{data['title']}/)
-        expect(formatted).to match(/#{data['image']}/)
-        expect(formatted).to match(/#{data['description']}/)
-      end
-    end
-  end
-end

spec/views/status_messages/_status_message.mobile.haml_spec.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+describe "status_messages/_status_message.mobile.haml" do
+  it "escapes the OpenGraph metadata" do
+    open_graph_cache = OpenGraphCache.new(
+      url:         "<script>alert(0);</script>",
+      title:       "<script>alert(0);</script>",
+      image:       "https://example.org/\"><script>alert(0);</script>",
+      description: "<script>alert(0);</script>"
+    )
+    post = FactoryGirl.create(:status_message, public: true, open_graph_cache: open_graph_cache)
+
+    render file: "status_messages/_status_message.mobile.haml", locals: {post: post, photos: post.photos}
+
+    expect(rendered).to_not include("<script>")
+  end
+end