Merge branch 'security-stored-xss-using-find-file' into 'master'

Sanitize search text to prevent XSS

See merge request gitlab/gitlabhq!3453

app/assets/javascripts/project_find_file.js

@@ -5,6 +5,7 @@ import fuzzaldrinPlus from 'fuzzaldrin-plus';
 import axios from '~/lib/utils/axios_utils';
 import flash from '~/flash';
 import { __ } from '~/locale';
+import sanitize from 'sanitize-html';
 
 // highlight text(awefwbwgtc -> <b>a</b>wefw<b>b</b>wgt<b>c</b> )
 const highlighter = function(element, text, matches) {
@@ -74,7 +75,7 @@ export default class ProjectFindFile {
 
   findFile() {
     var result, searchText;
-    searchText = this.inputElement.val();
+    searchText = sanitize(this.inputElement.val());
     result =
       searchText.length > 0 ? fuzzaldrinPlus.filter(this.filePaths, searchText) : this.filePaths;
     return this.renderList(result, searchText);

changelogs/unreleased/security-stored-xss-using-find-file.yml

+---
+title: Sanitize search text to prevent XSS
+merge_request:
+author:
+type: security

spec/frontend/project_find_file_spec.js

@@ -3,6 +3,9 @@ import $ from 'jquery';
 import ProjectFindFile from '~/project_find_file';
 import axios from '~/lib/utils/axios_utils';
 import { TEST_HOST } from 'helpers/test_constants';
+import sanitize from 'sanitize-html';
+
+jest.mock('sanitize-html', () => jest.fn(val => val));
 
 const BLOB_URL_TEMPLATE = `${TEST_HOST}/namespace/project/blob/master`;
 const FILE_FIND_URL = `${TEST_HOST}/namespace/project/files/master?format=json`;
@@ -38,31 +41,31 @@ describe('ProjectFindFile', () => {
         href: el.querySelector('a').href,
       }));
 
+  const files = [
+    'fileA.txt',
+    'fileB.txt',
+    'fi#leC.txt',
+    'folderA/fileD.txt',
+    'folder#B/fileE.txt',
+    'folde?rC/fil#F.txt',
+  ];
+
   beforeEach(() => {
     // Create a mock adapter for stubbing axios API requests
     mock = new MockAdapter(axios);
 
     element = $(TEMPLATE);
+    mock.onGet(FILE_FIND_URL).replyOnce(200, files);
+    getProjectFindFileInstance(); // This triggers a load / axios call + subsequent render in the constructor
   });
 
   afterEach(() => {
     // Reset the mock adapter
     mock.restore();
+    sanitize.mockClear();
   });
 
   it('loads and renders elements from remote server', done => {
-    const files = [
-      'fileA.txt',
-      'fileB.txt',
-      'fi#leC.txt',
-      'folderA/fileD.txt',
-      'folder#B/fileE.txt',
-      'folde?rC/fil#F.txt',
-    ];
-    mock.onGet(FILE_FIND_URL).replyOnce(200, files);
-
-    getProjectFindFileInstance(); // This triggers a load / axios call + subsequent render in the constructor
-
     setImmediate(() => {
       expect(findFiles()).toEqual(
         files.map(text => ({
@@ -74,4 +77,14 @@ describe('ProjectFindFile', () => {
       done();
     });
   });
+
+  it('sanitizes search text', done => {
+    const searchText = element.find('.file-finder-input').val();
+
+    setImmediate(() => {
+      expect(sanitize).toHaveBeenCalledTimes(1);
+      expect(sanitize).toHaveBeenCalledWith(searchText);
+      done();
+    });
+  });
 });