Merge branch 'security-215640-pypi' into 'master'

Fixes pypi XSS Closes #141 See merge request gitlab-org/security/gitlab!555

Merge branch 'security-215640-pypi' into 'master'
Fixes pypi XSS Closes #141 See merge request gitlab-org/security/gitlab!555
345ab151 · GitLab Release Tools Bot · 48b6f793 · bbf010e4 · 345ab151 · 345ab151
Commit 345ab151 authored Jun 29, 2020 by GitLab Release Tools Bot
3 changed files
--- a/ee/app/presenters/packages/pypi/package_presenter.rb
+++ b/ee/app/presenters/packages/pypi/package_presenter.rb
@@ -20,10 +20,10 @@ module Packages
        <!DOCTYPE html>
        <html>
          <head>
-            <title>Links for #{name}</title>
+            <title>Links for #{escape(name)}</title>
          </head>
          <body>
-            <h1>Links for #{name}</h1>
+            <h1>Links for #{escape(name)}</h1>
            #{links}
          </body>
        </html>
@@ -47,7 +47,7 @@ module Packages
      end

      def package_link(url, required_python, filename)
-        "<a href=\"#{url}\" data-requires-python=\"#{required_python}\">#{filename}</a><br>"
+        "<a href=\"#{url}\" data-requires-python=\"#{escape(required_python)}\">#{filename}</a><br>"
      end

      def build_pypi_package_path(file)
@@ -66,6 +66,10 @@ module Packages
      def name
        @packages.first.name
      end
+
+      def escape(str)
+        ERB::Util.html_escape(str)
+      end
    end
  end
 end
--- a/ee/changelogs/unreleased/security-215640-pypi.yml
+++ b/ee/changelogs/unreleased/security-215640-pypi.yml
+---
+title: Fixed pypi package API XSS
+merge_request:
+author:
+type: security
--- a/ee/spec/presenters/packages/pypi/package_presenter_spec.rb
+++ b/ee/spec/presenters/packages/pypi/package_presenter_spec.rb
@@ -19,16 +19,30 @@ RSpec.describe ::Packages::Pypi::PackagePresenter do
    shared_examples_for "pypi package presenter" do
      let(:file) { package.package_files.first }
      let(:filename) { file.file_name }
-      let(:expected_file) { "<a href=\"http://localhost/api/v4/projects/#{project.id}/packages/pypi/files/#{file.file_sha256}/#{filename}#sha256=#{file.file_sha256}\" data-requires-python=\"#{package.pypi_metadatum.required_python}\">#{filename}</a><br>" }
+      let(:expected_file) { "<a href=\"http://localhost/api/v4/projects/#{project.id}/packages/pypi/files/#{file.file_sha256}/#{filename}#sha256=#{file.file_sha256}\" data-requires-python=\"#{expected_python_version}\">#{filename}</a><br>" }
+
+      before do
+        package.pypi_metadatum.required_python = python_version
+      end

      it { is_expected.to include expected_file }
    end

    it_behaves_like "pypi package presenter" do
+      let(:python_version) { '>=2.7' }
+      let(:expected_python_version) { '&gt;=2.7' }
+      let(:package) { package1 }
+    end
+
+    it_behaves_like "pypi package presenter" do
+      let(:python_version) { '"><script>alert(1)</script>' }
+      let(:expected_python_version) { '&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;' }
      let(:package) { package1 }
    end

    it_behaves_like "pypi package presenter" do
+      let(:python_version) { '>=2.7, !=3.0' }
+      let(:expected_python_version) { '&gt;=2.7, !=3.0' }
      let(:package) { package2 }
    end
  end