Merge remote-tracking branch 'dev/master'

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 10.8.2 (2018-05-28)
+
+### Security (3 changes)
+
+- Fixed XSS in protected branches & tags access dropdown.
+- Escape name in merge request approvers dropdown.
+- Fixes include directive to not allow SSRF requests.
+
+
 ## 10.8.1 (2018-05-23)
 
 ### Fixed (4 changes)
@@ -88,6 +97,15 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Remove `features/group_active_tab.feature`. !5554 (@blackst0ne)
 
 
+## 10.7.5 (2018-05-28)
+
+### Security (3 changes)
+
+- Fixed XSS in protected branches & tags access dropdown.
+- Escape name in merge request approvers dropdown.
+- Fixes include directive to not allow SSRF requests.
+
+
 ## 10.7.4 (2018-05-21)
 
 ### Fixed (2 changes)
@@ -197,6 +215,15 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Breaks utils function to parse codeclimate and sast into separate functions.
 
 
+## 10.6.6 (2018-05-28)
+
+### Security (3 changes)
+
+- Fixed XSS in protected branches & tags access dropdown.
+- Escape name in merge request approvers dropdown.
+- Fixes include directive to not allow SSRF requests.
+
+
 ## 10.6.5 (2018-04-24)
 
 - No changes.

CHANGELOG.md

@@ -2,6 +2,15 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 10.8.2 (2018-05-28)
+
+### Security (3 changes)
+
+- Prevent user passwords from being changed without providing the previous password.
+- Fix API to remove deploy key from project instead of deleting it entirely.
+- Fixed bug that allowed importing arbitrary project attributes.
+
+
 ## 10.8.1 (2018-05-23)
 
 ### Fixed (9 changes)
@@ -193,6 +202,15 @@ entry.
 - Gitaly handles repository forks by default.
 
 
+## 10.7.5 (2018-05-28)
+
+### Security (3 changes)
+
+- Prevent user passwords from being changed without providing the previous password.
+- Fix API to remove deploy key from project instead of deleting it entirely.
+- Fixed bug that allowed importing arbitrary project attributes.
+
+
 ## 10.7.4 (2018-05-21)
 
 ### Fixed (1 change)
@@ -457,6 +475,16 @@ entry.
 - Upgrade Gitaly to upgrade its charlock_holmes.
 
 
+## 10.6.6 (2018-05-28)
+
+### Security (4 changes)
+
+- Do not allow non-members to create MRs via forked projects when MRs are private.
+- Prevent user passwords from being changed without providing the previous password.
+- Fix API to remove deploy key from project instead of deleting it entirely.
+- Fixed bug that allowed importing arbitrary project attributes.
+
+
 ## 10.6.5 (2018-04-24)
 
 ### Security (1 change)

ee/app/assets/javascripts/approvers_select.js

 import $ from 'jquery';
+import _ from 'underscore';
 import Api from '~/api';
 import { __ } from '~/locale';
 import Flash from '~/flash';
@@ -29,10 +30,9 @@ export default class ApproversSelect {
 
   static getApprovers(fieldName, approverList) {
     const input = $(`[name="${fieldName}"]`);
-    const existingApprovers = $(approverList).map((i, el) =>
-      parseInt($(el).data('id'), 10),
-    );
-    const selectedApprovers = input.val()
+    const existingApprovers = $(approverList).map((i, el) => parseInt($(el).data('id'), 10));
+    const selectedApprovers = input
+      .val()
       .split(',')
       .filter(val => val !== '');
     return [...existingApprovers, ...selectedApprovers];
@@ -76,40 +76,41 @@ export default class ApproversSelect {
   }
 
   initSelect2() {
-    this.$approverSelect.select2({
-      placeholder: 'Search for users or groups',
-      multiple: true,
-      minimumInputLength: 0,
-      query: (query) => {
-        const fetchGroups = this.fetchGroups(query.term);
-        const fetchUsers = this.fetchUsers(query.term);
-        return Promise.all([fetchGroups, fetchUsers]).then(([groups, users]) => {
-          const data = {
-            results: groups.concat(users),
+    this.$approverSelect
+      .select2({
+        placeholder: 'Search for users or groups',
+        multiple: true,
+        minimumInputLength: 0,
+        query: query => {
+          const fetchGroups = this.fetchGroups(query.term);
+          const fetchUsers = this.fetchUsers(query.term);
+          return Promise.all([fetchGroups, fetchUsers]).then(([groups, users]) => {
+            const data = {
+              results: groups.concat(users),
+            };
+            return query.callback(data);
+          });
+        },
+        formatResult: ApproversSelect.formatResult,
+        formatSelection: ApproversSelect.formatSelection,
+        dropdownCss() {
+          const $input = $('.js-select-user-and-group .select2-input');
+          const offset = $input.offset();
+          const inputRightPosition = offset.left + $input.outerWidth();
+          const $dropdown = $('.select2-drop-active');
+
+          let left = offset.left;
+          if ($dropdown.outerWidth() > $input.outerWidth()) {
+            left = `${inputRightPosition - $dropdown.width()}px`;
+          }
+          return {
+            left,
+            right: 'auto',
+            width: 'auto',
           };
-          return query.callback(data);
-        });
-      },
-      formatResult: ApproversSelect.formatResult,
-      formatSelection: ApproversSelect.formatSelection,
-      dropdownCss() {
-        const $input = $('.js-select-user-and-group .select2-input');
-        const offset = $input.offset();
-        const inputRightPosition = offset.left + $input.outerWidth();
-        const $dropdown = $('.select2-drop-active');
-
-        let left = offset.left;
-        if ($dropdown.outerWidth() > $input.outerWidth()) {
-          left = `${inputRightPosition - $dropdown.width()}px`;
-        }
-        return {
-          left,
-          right: 'auto',
-          width: 'auto',
-        };
-      },
-    })
-    .on('change', this.handleSelectChange);
+        },
+      })
+      .on('change', this.handleSelectChange);
   }
 
   static formatSelection(group) {
@@ -131,8 +132,8 @@ export default class ApproversSelect {
             <img class="avatar s40" src="${avatar}">
           </div>
           <div class="user-info">
-            <div class="user-name">${name}</div>
-            <div class="user-username">@${username}</div>
+            <div class="user-name">${_.escape(name)}</div>
+            <div class="user-username">@${_.escape(username)}</div>
           </div>
         </div>
       `;
@@ -140,8 +141,8 @@ export default class ApproversSelect {
 
     return `
       <div class="group-result">
-        <div class="group-name">${fullName}</div>
-        <div class="group-path">${fullPath}</div>
+        <div class="group-name">${_.escape(fullName)}</div>
+        <div class="group-path">${_.escape(fullPath)}</div>
       </div>
     `;
   }
@@ -163,17 +164,20 @@ export default class ApproversSelect {
     const $form = $('.js-add-approvers').closest('form');
     $loadWrapper.removeClass('hidden');
 
-    axios.post($form.attr('action'), `_method=PATCH&${[encodeURIComponent(fieldName)]}=${newValue}`, {
-      headers: {
-        'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8',
-      },
-    }).then(({ data }) => {
-      ApproversSelect.updateApproverList(data);
-      ApproversSelect.saveApproversComplete($input, $approverSelect, $loadWrapper);
-    }).catch(() => {
-      Flash(__('An error occurred while adding approver'));
-      ApproversSelect.saveApproversComplete($input, $approverSelect, $loadWrapper);
-    });
+    axios
+      .post($form.attr('action'), `_method=PATCH&${[encodeURIComponent(fieldName)]}=${newValue}`, {
+        headers: {
+          'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8',
+        },
+      })
+      .then(({ data }) => {
+        ApproversSelect.updateApproverList(data);
+        ApproversSelect.saveApproversComplete($input, $approverSelect, $loadWrapper);
+      })
+      .catch(() => {
+        Flash(__('An error occurred while adding approver'));
+        ApproversSelect.saveApproversComplete($input, $approverSelect, $loadWrapper);
+      });
   }
 
   static saveApproversComplete($input, $approverSelect, $loadWrapper) {
@@ -188,20 +192,27 @@ export default class ApproversSelect {
     const $loadWrapper = $('.load-wrapper');
     $loadWrapper.removeClass('hidden');
 
-    axios.post(target.getAttribute('href'), '_method=DELETE', {
-      headers: {
-        'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8',
-      },
-    }).then(({ data }) => {
-      ApproversSelect.updateApproverList(data);
-      $loadWrapper.addClass('hidden');
-    }).catch(() => {
-      Flash(__('An error occurred while removing approver'));
-      $loadWrapper.addClass('hidden');
-    });
+    axios
+      .post(target.getAttribute('href'), '_method=DELETE', {
+        headers: {
+          'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8',
+        },
+      })
+      .then(({ data }) => {
+        ApproversSelect.updateApproverList(data);
+        $loadWrapper.addClass('hidden');
+      })
+      .catch(() => {
+        Flash(__('An error occurred while removing approver'));
+        $loadWrapper.addClass('hidden');
+      });
   }
 
   static updateApproverList(html) {
-    $('.js-current-approvers').html($(html).find('.js-current-approvers').html());
+    $('.js-current-approvers').html(
+      $(html)
+        .find('.js-current-approvers')
+        .html(),
+    );
   }
 }

ee/app/assets/javascripts/projects/settings/access_dropdown.js

@@ -458,7 +458,7 @@ export default class AccessDropdown {
       <li>
         <a href="#" class="${isActiveClass}">
           <img src="${user.avatar_url}" class="avatar avatar-inline" width="30">
-          <strong class="dropdown-menu-user-full-name">${user.name}</strong>
+          <strong class="dropdown-menu-user-full-name">${_.escape(user.name)}</strong>
           <span class="dropdown-menu-user-username">${user.username}</span>
         </a>
       </li>

ee/changelogs/unreleased/security-approvers-security-fix-master.yml

+---
+title: Escape name in merge request approvers dropdown
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-include-directive-allows-ssrf-requests.yml

+---
+title: Fixes include directive to not allow SSRF requests
+merge_request:
+author:
+type: security

ee/lib/gitlab/ci/external/file/remote.rb

@@ -11,8 +11,8 @@ module Gitlab
 
             @content = strong_memoize(:content) do
               begin
-                Gitlab::HTTP.get(location, allow_local_requests: true)
-              rescue Gitlab::HTTP::Error, Timeout::Error, SocketError
+                Gitlab::HTTP.get(location)
+              rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError
                 nil
               end
             end

ee/spec/lib/gitlab/ci/external/file/remote_spec.rb

@@ -50,6 +50,14 @@ describe Gitlab::Ci::External::File::Remote do
         expect(remote_file.valid?).to be_falsy
       end
     end
+
+    context 'with an internal url' do
+      let(:location) { 'http://localhost:8080' }
+
+      it 'should be falsy' do
+        expect(remote_file.valid?).to be_falsy
+      end
+    end
   end
 
   describe "#content" do
@@ -84,6 +92,14 @@ describe Gitlab::Ci::External::File::Remote do
         expect(remote_file.content).to be_nil
       end
     end
+
+    context 'with an internal url' do
+      let(:location) { 'http://localhost:8080' }
+
+      it 'should be nil' do
+        expect(remote_file.content).to be_nil
+      end
+    end
   end
 
   describe "#error_message" do

spec/javascripts/approvers_select_spec.js

@@ -34,4 +34,29 @@ describe('ApproversSelect', () => {
       expect($loadWrapper.addClass).toHaveBeenCalledWith('hidden');
     });
   });
+
+  describe('formatResult', () => {
+    it('escapes name', () => {
+      const output = ApproversSelect.formatResult({
+        name: '<script>alert("testing")</script>',
+        username: 'testing',
+        avatar_url: gl.TEST_HOST,
+        full_name: '<script>alert("testing")</script>',
+        full_path: 'testing',
+      });
+
+      expect(output).not.toContain('<script>alert("testing")</script>');
+    });
+
+    it('escapes full name', () => {
+      const output = ApproversSelect.formatResult({
+        username: 'testing',
+        avatar_url: gl.TEST_HOST,
+        full_name: '<script>alert("testing")</script>',
+        full_path: 'testing',
+      });
+
+      expect(output).not.toContain('<script>alert("testing")</script>');
+    });
+  });
 });

spec/javascripts/ee/projects/settings/access_dropdown_spec.js

@@ -123,4 +123,17 @@ describe('AccessDropdown', () => {
       });
     });
   });
+
+  describe('userRowHtml', () => {
+    it('escapes users name', () => {
+      const user = {
+        avatar_url: '',
+        name: '<img src=x onerror=alert(document.domain)>',
+        username: 'test',
+      };
+      const template = dropdown.userRowHtml(user);
+
+      expect(template).not.toContain(user.name);
+    });
+  });
 });