SECURITY: Limit the length of drafts (#19989)

Co-authored-by: Loïc Guitaut <loic@discourse.org>

app/controllers/drafts_controller.rb

@@ -24,6 +24,16 @@ class DraftsController < ApplicationController
   def create
     raise Discourse::NotFound.new if params[:draft_key].blank?
 
+    if params[:data].size > SiteSetting.max_draft_length
+      raise Discourse::InvalidParameters.new(:data)
+    end
+
+    begin
+      data = JSON.parse(params[:data])
+    rescue JSON::ParserError
+      raise Discourse::InvalidParameters.new(:data)
+    end
+
     sequence =
       begin
         Draft.set(
@@ -59,12 +69,6 @@ class DraftsController < ApplicationController
 
     json = success_json.merge(draft_sequence: sequence)
 
-    begin
-      data = JSON.parse(params[:data])
-    rescue JSON::ParserError
-      raise Discourse::InvalidParameters.new(:data)
-    end
-
     if data.present?
       # this is a bit of a kludge we need to remove (all the parsing) too many special cases here
       # we need to catch action edit and action editSharedDraft

config/site_settings.yml

@@ -1090,6 +1090,9 @@ posting:
   skip_auto_delete_reply_likes: 5
   review_every_post:
     default: false
+  max_draft_length:
+    default: 400_000
+    hidden: true
 
 email:
   email_time_window_mins:

db/post_migrate/20230117143451_drop_invalid_drafts.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class DropInvalidDrafts < ActiveRecord::Migration[7.0]
+  def up
+    execute <<~SQL
+    DELETE FROM drafts
+    WHERE LENGTH(data) > 400000
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/requests/drafts_controller_spec.rb

@@ -192,6 +192,40 @@ RSpec.describe DraftsController do
 
       expect(response.status).to eq(409)
     end
+
+    context "when data is too big" do
+      let(:user) { Fabricate(:user) }
+      let(:data) { "a" * (SiteSetting.max_draft_length + 1) }
+
+      before do
+        SiteSetting.max_draft_length = 500
+        sign_in(user)
+      end
+
+      it "returns an error" do
+        post "/drafts.json",
+             params: {
+               draft_key: "xyz",
+               data: { reply: data }.to_json,
+               sequence: 0,
+             }
+        expect(response).to have_http_status :bad_request
+      end
+    end
+
+    context "when data is not too big" do
+      context "when data is not proper JSON" do
+        let(:user) { Fabricate(:user) }
+        let(:data) { "not-proper-json" }
+
+        before { sign_in(user) }
+
+        it "returns an error" do
+          post "/drafts.json", params: { draft_key: "xyz", data: data, sequence: 0 }
+          expect(response).to have_http_status :bad_request
+        end
+      end
+    end
   end
 
   describe "#destroy" do