Merge branch 'security-package-json-xss' into 'master'

[master] Fix XSS vulnerability sourced from package.json's homepage

Closes #2702

See merge request gitlab/gitlabhq!2496

3 files changed, 24 insertions, 5 deletions

diff --git a/app/models/blob_viewer/package_json.rb b/app/models/blob_viewer/package_json.rb
index d12dd93ce2e..7cae60a74d6 100644
--- a/app/models/blob_viewer/package_json.rb
+++ b/app/models/blob_viewer/package_json.rb
@@ -33,7 +33,8 @@ module BlobViewer
     end
 
     def homepage
-      json_data['homepage']
+      url = json_data['homepage']
+      url if Gitlab::UrlSanitizer.valid?(url)
     end
 
     def npm_url
diff --git a/changelogs/unreleased/security-package-json-xss.yml b/changelogs/unreleased/security-package-json-xss.yml
new file mode 100644
index 00000000000..6ab4854e44f
--- /dev/null
+++ b/changelogs/unreleased/security-package-json-xss.yml
@@ -0,0 +1,5 @@
+---
+title: Fix xss vulnerability sourced from package.json
+merge_request:
+author:
+type: security
diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb
index 5ed2f4400bc..fbaa8d47a71 100644
--- a/spec/models/blob_viewer/package_json_spec.rb
+++ b/spec/models/blob_viewer/package_json_spec.rb
@@ -40,13 +40,14 @@ describe BlobViewer::PackageJson do
   end
 
   context 'when package.json has "private": true' do
+    let(:homepage) { 'http://example.com' }
     let(:data) do
       <<-SPEC.strip_heredoc
       {
         "name": "module-name",
         "version": "10.3.1",
         "private": true,
-        "homepage": "myawesomepackage.com"
+        "homepage": #{homepage.to_json}
       }
       SPEC
     end
@@ -54,10 +55,22 @@ describe BlobViewer::PackageJson do
     subject { described_class.new(blob) }
 
     describe '#package_url' do
-      it 'returns homepage if any' do
-        expect(subject).to receive(:prepare!)
+      context 'when the homepage has a valid URL' do
+        it 'returns homepage URL' do
+          expect(subject).to receive(:prepare!)
+
+          expect(subject.package_url).to eq(homepage)
+        end
+      end
+
+      context 'when the homepage has an invalid URL' do
+        let(:homepage) { 'javascript:alert()' }
+
+        it 'returns nil' do
+          expect(subject).to receive(:prepare!)
 
-        expect(subject.package_url).to eq('myawesomepackage.com')
+          expect(subject.package_url).to be_nil
+        end
       end
     end