From 6ebffaaf6e929523e81168fefc55385cbb75f5ef Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <mail@gerhard-schlager.at>
Date: Mon, 11 Nov 2019 22:30:31 +0100
Subject: [PATCH] FIX: Better error handling for invalid locale bundle versions

---
 app/controllers/extra_locales_controller.rb    | 13 +++++++++----
 spec/requests/extra_locales_controller_spec.rb |  8 ++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/app/controllers/extra_locales_controller.rb b/app/controllers/extra_locales_controller.rb
index 4bee4449764..e20c81ea424 100644
--- a/app/controllers/extra_locales_controller.rb
+++ b/app/controllers/extra_locales_controller.rb
@@ -9,15 +9,20 @@ class ExtraLocalesController < ApplicationController
     :verify_authenticity_token
 
   OVERRIDES_BUNDLE ||= 'overrides'
+  MD5_HASH_LENGTH ||= 32
 
   def show
     bundle = params[:bundle]
-
     raise Discourse::InvalidAccess.new if !valid_bundle?(bundle)
 
-    if params[:v]&.size == 32
-      hash = ExtraLocalesController.bundle_js_hash(bundle)
-      immutable_for(1.year) if hash == params[:v]
+    version = params[:v]
+    if version.present?
+      if version.kind_of?(String) && version.length == MD5_HASH_LENGTH
+        hash = ExtraLocalesController.bundle_js_hash(bundle)
+        immutable_for(1.year) if hash == version
+      else
+        raise Discourse::InvalidParameters.new(:v)
+      end
     end
 
     render plain: ExtraLocalesController.bundle_js(bundle), content_type: "application/javascript"
diff --git a/spec/requests/extra_locales_controller_spec.rb b/spec/requests/extra_locales_controller_spec.rb
index 7c12b1fcfdb..d46f6c71efa 100644
--- a/spec/requests/extra_locales_controller_spec.rb
+++ b/spec/requests/extra_locales_controller_spec.rb
@@ -23,6 +23,14 @@ describe ExtraLocalesController do
       expect(response.status).to eq(403)
     end
 
+    it "requires a valid version" do
+      get "/extra-locales/overrides", params: { v: 'a' }
+      expect(response.status).to eq(400)
+
+      get "/extra-locales/overrides?v[foo]=1"
+      expect(response.status).to eq(400)
+    end
+
     context "logged in as a moderator" do
 
       let(:moderator) { Fabricate(:moderator) }