From 32cd764786ed33c50765ef213ddef8f1d0897353 Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Thu, 8 Nov 2018 19:49:56 +0100
Subject: [PATCH] Fix XSS via OpenGraph on mobile

---
 Changelog.md                                  |  4 +++
 app/helpers/open_graph_helper.rb              | 10 ------
 .../_status_message.mobile.haml               |  8 ++++-
 spec/helpers/open_graph_helper_spec.rb        | 32 -------------------
 .../_status_message.mobile.haml_spec.rb       | 17 ++++++++++
 5 files changed, 28 insertions(+), 43 deletions(-)
 delete mode 100644 spec/helpers/open_graph_helper_spec.rb
 create mode 100644 spec/views/status_messages/_status_message.mobile.haml_spec.rb

diff --git a/Changelog.md b/Changelog.md
index af6473929..44bbc3064 100644
--- a/Changelog.md
+++ b/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
diff --git a/app/helpers/open_graph_helper.rb b/app/helpers/open_graph_helper.rb
index ace0990c8..383a2c69c 100644
--- a/app/helpers/open_graph_helper.rb
+++ b/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
diff --git a/app/views/status_messages/_status_message.mobile.haml b/app/views/status_messages/_status_message.mobile.haml
index 1758a96cc..bf5fe21dd 100644
--- a/app/views/status_messages/_status_message.mobile.haml
+++ b/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: " ")
diff --git a/spec/helpers/open_graph_helper_spec.rb b/spec/helpers/open_graph_helper_spec.rb
deleted file mode 100644
index c6b10188a..000000000
--- a/spec/helpers/open_graph_helper_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/views/status_messages/_status_message.mobile.haml_spec.rb b/spec/views/status_messages/_status_message.mobile.haml_spec.rb
new file mode 100644
index 000000000..5487d6a55
--- /dev/null
+++ b/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