Fix HandleMalformedStrings middleware

Because of middleware loading order, sometimes we'd load before the
path has been unescaped. Here we change the loading order to be as early
as possible but also remove brittleness by unescaping the bits we care
about ourselves.

lib/gitlab/middleware/handle_malformed_strings.rb

@@ -26,13 +26,20 @@ module Gitlab
 
         request = Rack::Request.new(request)
 
-        return true if string_malformed?(request.path)
+        return true if malformed_path?(request.path)
 
         request.params.values.any? do |value|
           param_has_null_byte?(value)
         end
       end
 
+      def malformed_path?(path)
+        string_malformed?(Rack::Utils.unescape(path))
+      rescue ArgumentError
+        # Rack::Utils.unescape raised this, path is malformed.
+        true
+      end
+
       def param_has_null_byte?(value, depth = 0)
         # Guard against possible attack sending large amounts of nested params
         # Should be safe as deeply nested params are highly uncommon.

spec/lib/gitlab/middleware/handle_malformed_strings_spec.rb

@@ -5,7 +5,9 @@ require "rack/test"
 
 RSpec.describe Gitlab::Middleware::HandleMalformedStrings do
   let(:null_byte) { "\u0000" }
+  let(:escaped_null_byte) { "%00" }
   let(:invalid_string) { "mal\xC0formed" }
+  let(:escaped_invalid_string) { "mal%c0formed" }
   let(:error_400) { [400, { 'Content-Type' => 'text/plain' }, ['Bad Request']] }
   let(:app) { double(:app) }
 
@@ -30,6 +32,14 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do
       expect(subject.call(env)).to eq error_400
     end
 
+    it 'rejects escaped null bytes' do
+      # We have to create the env separately or Rack::MockRequest complains about invalid URI
+      env = env_for
+      env['PATH_INFO'] = "/someplace/withan#{escaped_null_byte}escaped nullbyte"
+
+      expect(subject.call(env)).to eq error_400
+    end
+
     it 'rejects malformed strings' do
       # We have to create the env separately or Rack::MockRequest complains about invalid URI
       env = env_for
@@ -37,6 +47,14 @@ RSpec.describe Gitlab::Middleware::HandleMalformedStrings do
 
       expect(subject.call(env)).to eq error_400
     end
+
+    it 'rejects escaped malformed strings' do
+      # We have to create the env separately or Rack::MockRequest complains about invalid URI
+      env = env_for
+      env['PATH_INFO'] = "/someplace/with_an/#{escaped_invalid_string}"
+
+      expect(subject.call(env)).to eq error_400
+    end
   end
 
   context 'in params' do