From 9cbbaf4237e2343348f5d0a7375796e6712b4580 Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Wed, 22 Apr 2020 13:50:45 -0600
Subject: [PATCH] FIX: Don't throw 500 for invalid website url input

It's possible to cause a 500 error by putting in weird characters in the
input field for updating a users website on their profile.

Normal invalid input like not including the domain extension is already
handled by the user_profile model validation. This fix ensures a server
error doesn't occur for weird input characters.
---
 app/services/user_updater.rb       | 35 +++++++++++++++++-------------
 spec/services/user_updater_spec.rb |  9 ++++++++
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb
index 5b3396f1017..cdd5203f969 100644
--- a/app/services/user_updater.rb
+++ b/app/services/user_updater.rb
@@ -149,25 +149,30 @@ class UserUpdater
 
     saved = nil
 
-    User.transaction do
-      if attributes.key?(:muted_usernames)
-        update_muted_users(attributes[:muted_usernames])
-      end
+    begin
+      User.transaction do
+        if attributes.key?(:muted_usernames)
+          update_muted_users(attributes[:muted_usernames])
+        end
 
-      if attributes.key?(:ignored_usernames)
-        update_ignored_users(attributes[:ignored_usernames])
-      end
+        if attributes.key?(:ignored_usernames)
+          update_ignored_users(attributes[:ignored_usernames])
+        end
 
-      name_changed = user.name_changed?
-      if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
-         (name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0)
+        name_changed = user.name_changed?
+        if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
+           (name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0)
 
-        StaffActionLogger.new(@actor).log_name_change(
-          user.id,
-          old_user_name,
-          attributes.fetch(:name) { '' }
-        )
+          StaffActionLogger.new(@actor).log_name_change(
+            user.id,
+            old_user_name,
+            attributes.fetch(:name) { '' }
+          )
+        end
       end
+    rescue Addressable::URI::InvalidURIError => e
+      # Prevent 500 for crazy url input
+      return saved
     end
 
     DiscourseEvent.trigger(:user_updated, user) if saved
diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb
index 5ca012fc135..c18949ab5e7 100644
--- a/spec/services/user_updater_spec.rb
+++ b/spec/services/user_updater_spec.rb
@@ -416,6 +416,15 @@ describe UserUpdater do
       end
     end
 
+    context 'when website is invalid' do
+      it 'returns an error' do
+        user = Fabricate(:user)
+        updater = UserUpdater.new(acting_user, user)
+
+        expect(updater.update(website: 'ʔ<')).to eq nil
+      end
+    end
+
     context 'when custom_fields is empty string' do
       it "update is successful" do
         user = Fabricate(:user)