Merge branch 'security-fix-invalid-byte-sequence-upload-links-ee-master' into 'master'

Fix invalid byte sequence for uploads links

See merge request gitlab-org/security/gitlab!28

changelogs/unreleased/security-fix-invalid-byte-sequence-upload-links-master.yml

+---
+title: Fix 500 error caused by invalid byte sequences in uploads links
+merge_request:
+author:
+type: security

lib/banzai/filter/relative_link_filter.rb

@@ -116,7 +116,7 @@ module Banzai
       end
 
       def process_link_to_upload_attr(html_attr)
-        path_parts = [Addressable::URI.unescape(html_attr.value)]
+        path_parts = [unescape_and_scrub_uri(html_attr.value)]
 
         if project
           path_parts.unshift(relative_url_root, project.full_path)
@@ -172,7 +172,7 @@ module Banzai
       end
 
       def cleaned_file_path(uri)
-        Addressable::URI.unescape(uri.path).scrub.delete("\0").chomp("/")
+        unescape_and_scrub_uri(uri.path).delete("\0").chomp("/")
       end
 
       def relative_file_path(uri)
@@ -184,7 +184,7 @@ module Banzai
       def request_path
         return unless context[:requested_path]
 
-        Addressable::URI.unescape(context[:requested_path]).chomp("/")
+        unescape_and_scrub_uri(context[:requested_path]).chomp("/")
       end
 
       # Convert a relative path into its correct location based on the currently
@@ -266,6 +266,12 @@ module Banzai
       def repository
         @repository ||= project&.repository
       end
+
+      private
+
+      def unescape_and_scrub_uri(uri)
+        Addressable::URI.unescape(uri).scrub
+      end
     end
   end
 end

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -128,6 +128,15 @@ describe Banzai::Filter::RelativeLinkFilter do
     expect { filter(act) }.not_to raise_error
   end
 
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in uploads' do
+    act = link("/uploads/%FF")
+    expect { filter(act) }.not_to raise_error
+  end
+
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in context requested path' do
+    expect { filter(link("files/test.md"), requested_path: '%FF') }.not_to raise_error
+  end
+
   it 'does not raise an exception with a garbled path' do
     act = link("open(/var/tmp/):%20/location%0Afrom:%20/test")
     expect { filter(act) }.not_to raise_error