From 382f6959fcc2319a00cfd5305c3d41acb8908af6 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Tue, 17 Sep 2019 16:12:50 -0400
Subject: [PATCH] SECURITY: XSS when oneboxing user profile location field

The XSS here is only possible if CSP is disabled. Low impact since CSP is enabled by default in SiteSettings.
---
 lib/oneboxer.rb                  |  2 +-
 spec/components/oneboxer_spec.rb | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index 94e9c40215..a4c9163804 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -245,7 +245,7 @@ module Oneboxer
         avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"),
         name: name,
         bio: user.user_profile.bio_excerpt(230),
-        location: user.user_profile.location,
+        location: Onebox::Helpers.sanitize(user.user_profile.location),
         joined: I18n.t('joined'),
         created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')),
         website: user.user_profile.website,
diff --git a/spec/components/oneboxer_spec.rb b/spec/components/oneboxer_spec.rb
index 5ed10343ce..659a975bcd 100644
--- a/spec/components/oneboxer_spec.rb
+++ b/spec/components/oneboxer_spec.rb
@@ -113,6 +113,25 @@ describe Oneboxer do
       expect(preview("#{path}.mov")).to include("<video ")
     end
 
+    it "strips HTML from user profile location" do
+      user = Fabricate(:user)
+      profile = user.reload.user_profile
+
+      expect(preview("/u/#{user.username}")).not_to include("<span class=\"location\">")
+
+      profile.update!(
+        location: "<img src=x onerror=alert(document.domain)>",
+      )
+
+      expect(preview("/u/#{user.username}")).to include("<span class=\"location\">")
+      expect(preview("/u/#{user.username}")).not_to include("<img src=x")
+
+      profile.update!(
+        location: "Thunderland",
+      )
+
+      expect(preview("/u/#{user.username}")).to include("Thunderland")
+    end
   end
 
   context ".onebox_raw" do
@@ -140,5 +159,4 @@ describe Oneboxer do
 
     expect(Oneboxer.external_onebox(url)[:onebox]).to be_present
   end
-
 end