Merge branch 'security-master-ee-xss-reports' into 'master'

[master] Fix XSS in Security Reports and License Management

See merge request gitlab/gitlab-ee!688

ee/app/assets/javascripts/vue_shared/components/is_safe_url.js

+/**
+ * Checks if the provided URL is a safe URL (absolute http(s) URL)
+ *
+ * @param {String} url that will be checked
+ * @returns {Boolean}
+ */
+export default url => {
+  let parsedUrl;
+
+  if (!(url.startsWith('https:') || url.startsWith('http:'))) {
+    return false;
+  }
+
+  /*
+   Trying to use URL constructor, IE11 does not support it, so we fall back on the a element trick
+   */
+  try {
+    parsedUrl = new URL(url);
+  } catch (e) {
+    parsedUrl = document.createElement('a');
+    parsedUrl.href = url;
+  }
+
+  return ['http:', 'https:'].includes(parsedUrl.protocol);
+};

ee/app/assets/javascripts/vue_shared/components/safe_link.vue

+<script>
+import isSafeURL from './is_safe_url';
+/**
+ * Renders a link element (`<a>`) if the href is a absolute http(s) URL,
+ * a `<span>` element otherwise
+ */
+export default {
+  name: 'SafeLink',
+  /*
+    The props contain all attributes specifically defined for the <a> element:
+    https://www.w3.org/TR/2011/WD-html5-20110113/text-level-semantics.html#the-a-element
+   */
+  props: {
+    href: {
+      type: String,
+      required: true,
+    },
+    target: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+    rel: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+    media: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+    hreflang: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+    type: {
+      type: String,
+      required: false,
+      default: undefined,
+    },
+  },
+  computed: {
+    hasSafeHref() {
+      return isSafeURL(this.href);
+    },
+    componentName() {
+      return this.hasSafeHref ? 'a' : 'span';
+    },
+    linkAttributes() {
+      if (this.hasSafeHref) {
+        const { href, target, rel, media, hreflang, type } = this;
+        return { href, target, rel, media, hreflang, type };
+      }
+      return {};
+    },
+  },
+};
+</script>
+<template>
+  <component
+    :is="componentName"
+    v-bind="linkAttributes"
+  >
+    <slot></slot>
+  </component>
+</template>

ee/app/assets/javascripts/vue_shared/license_management/components/set_approval_status_modal.vue

@@ -2,12 +2,13 @@
 import { s__ } from '~/locale';
 import { mapActions, mapState } from 'vuex';
 import GlModal from '~/vue_shared/components/gl_modal.vue';
+import SafeLink from 'ee/vue_shared/components/safe_link.vue';
 import LicensePackages from './license_packages.vue';
 import { LICENSE_APPROVAL_STATUS } from '../constants';
 
 export default {
   name: 'LicenseSetApprovalStatusModal',
-  components: { LicensePackages, GlModal },
+  components: { SafeLink, LicensePackages, GlModal },
   computed: {
     ...mapState(['currentLicenseInModal', 'canManageLicenses']),
     headerTitleText() {
@@ -63,11 +64,11 @@ export default {
           {{ s__('LicenseManagement|URL') }}:
         </label>
         <div class="col-sm-9 text-secondary">
-          <a
+          <safe-link
             :href="currentLicenseInModal.url"
             target="_blank"
             rel="noopener noreferrer nofollow"
-          >{{ currentLicenseInModal.url }}</a>
+          >{{ currentLicenseInModal.url }}</safe-link>
         </div>
       </div>
       <div class="row prepend-top-10 append-bottom-10 js-license-packages">

ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue

@@ -4,9 +4,11 @@ import Modal from '~/vue_shared/components/gl_modal.vue';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import Icon from '~/vue_shared/components/icon.vue';
 import ExpandButton from '~/vue_shared/components/expand_button.vue';
+import SafeLink from 'ee/vue_shared/components/safe_link.vue';
 
 export default {
   components: {
+    SafeLink,
     Modal,
     LoadingButton,
     ExpandButton,
@@ -122,14 +124,14 @@ export default {
                     {{ instance.method }}
                   </div>
                   <div class="report-block-list-issue-description-link">
-                    <a
+                    <safe-link
                       :href="instance.uri"
                       target="_blank"
                       rel="noopener noreferrer nofollow"
                       class="break-link"
                     >
                       {{ instance.uri }}
-                    </a>
+                    </safe-link>
                   </div>
                   <expand-button v-if="instance.evidence">
                     <pre
@@ -146,7 +148,7 @@ export default {
               v-for="(identifier, i) in field.value"
               :key="i"
             >
-              <a
+              <safe-link
                 v-if="identifier.url"
                 :class="`js-link-${key}`"
                 :href="identifier.url"
@@ -154,7 +156,7 @@ export default {
                 rel="noopener noreferrer"
               >
                 {{ identifier.name }}
-              </a>
+              </safe-link>
               <span v-else>
                 {{ identifier.name }}
               </span>
@@ -166,26 +168,26 @@ export default {
               v-for="(link, i) in field.value"
               :key="i"
             >
-              <a
+              <safe-link
                 :class="`js-link-${key}`"
                 :href="link.url"
                 target="_blank"
                 rel="noopener noreferrer"
               >
                 {{ link.value || link.url }}
-              </a>
+              </safe-link>
               <span v-if="isLastValue(i, field.value)">,&nbsp;</span>
             </span>
           </template>
           <template v-else>
-            <a
+            <safe-link
               v-if="field.isLink"
               :class="`js-link-${key}`"
               :href="field.url"
               target="_blank"
             >
               {{ field.value }}
-            </a>
+            </safe-link>
             <span v-else>
               {{ field.value }}
             </span>

ee/spec/javascripts/license_management/components/set_approval_status_modal_spec.js

@@ -303,4 +303,29 @@ describe('SetApprovalModal', () => {
       });
     });
   });
+
+  it('does not render a XSS link', done => {
+    // eslint-disable-next-line no-script-url
+    const badURL = 'javascript:alert("")';
+
+    store.replaceState({
+      currentLicenseInModal: {
+        ...licenseReport[0],
+        url: badURL,
+        approvalStatus: LICENSE_APPROVAL_STATUS.APPROVED,
+      },
+    });
+    Vue.nextTick()
+      .then(() => {
+        const licenseName = vm.$el.querySelector('.js-license-url');
+        expect(licenseName).not.toBeNull();
+        expect(trimText(licenseName.innerText)).toBe(`URL: ${badURL}`);
+
+        expect(licenseName.querySelector('a')).toBeNull();
+        expect(licenseName.querySelector('span')).not.toBeNull();
+        expect(licenseName.querySelector('span').innerText).toBe(badURL);
+      })
+      .then(done)
+      .catch(done.fail);
+  });
 });

ee/spec/javascripts/vue_shared/components/is_safe_url_spec.js

+/* eslint-disable no-script-url */
+import isSafeURL from 'ee/vue_shared/components/is_safe_url';
+
+describe('isSafeUrl', () => {
+  describe('with URL constructor support', () => {
+    it('returns true for absolute http(s) urls', () => {
+      expect(isSafeURL('http://example.org')).toBe(true);
+      expect(isSafeURL('http://example.org:8080')).toBe(true);
+      expect(isSafeURL('https://example.org')).toBe(true);
+      expect(isSafeURL('https://example.org:8080')).toBe(true);
+      expect(isSafeURL('https://192.168.1.1')).toBe(true);
+    });
+
+    it('returns false for relative urls', () => {
+      expect(isSafeURL('./relative/link')).toBe(false);
+      expect(isSafeURL('/relative/link')).toBe(false);
+      expect(isSafeURL('../relative/link')).toBe(false);
+    });
+
+    it('returns false for http(s) urls without host', () => {
+      expect(isSafeURL('http://')).toBe(false);
+      expect(isSafeURL('https://')).toBe(false);
+      expect(isSafeURL('https:https:https:')).toBe(false);
+    });
+
+    it('returns false for non http(s) links', () => {
+      expect(isSafeURL('javascript:')).toBe(false);
+      expect(isSafeURL('javascript:alert("XSS")')).toBe(false);
+      expect(isSafeURL('jav\tascript:alert("XSS");')).toBe(false);
+      expect(isSafeURL('   javascript:alert("XSS");')).toBe(false);
+      expect(isSafeURL('ftp://192.168.1.1')).toBe(false);
+      expect(isSafeURL('file:///')).toBe(false);
+      expect(isSafeURL('file:///etc/hosts')).toBe(false);
+    });
+
+    it('returns false for encoded javascript links', () => {
+      expect(
+        isSafeURL(
+          '&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          'javascript:alert('XSS')',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          '&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          '\\u006A\\u0061\\u0076\\u0061\\u0073\\u0063\\u0072\\u0069\\u0070\\u0074\\u003A\\u0061\\u006C\\u0065\\u0072\\u0074\\u0028\\u0027\\u0058\\u0053\\u0053\\u0027\\u0029',
+        ),
+      ).toBe(false);
+    });
+  });
+
+  describe('without URL constructor support', () => {
+    beforeEach(() => {
+      spyOn(window, 'URL').and.callFake(() => {
+        throw new Error('No URL support');
+      });
+    });
+
+    it('returns true for absolute http(s) urls', () => {
+      expect(isSafeURL('http://example.org')).toBe(true);
+      expect(isSafeURL('http://example.org:8080')).toBe(true);
+      expect(isSafeURL('https://example.org')).toBe(true);
+      expect(isSafeURL('https://example.org:8080')).toBe(true);
+      expect(isSafeURL('https://192.168.1.1')).toBe(true);
+    });
+
+    it('returns true for relative urls', () => {
+      expect(isSafeURL('./relative/link')).toBe(false);
+      expect(isSafeURL('/relative/link')).toBe(false);
+      expect(isSafeURL('../relative/link')).toBe(false);
+    });
+
+    it('returns false for http(s) urls without host', () => {
+      expect(isSafeURL('http://')).toBe(false);
+      expect(isSafeURL('https://')).toBe(false);
+      expect(isSafeURL('https:https:https:')).toBe(false);
+    });
+
+    it('returns false for non http(s) links', () => {
+      expect(isSafeURL('javascript:')).toBe(false);
+      expect(isSafeURL('javascript:alert("XSS")')).toBe(false);
+      expect(isSafeURL('jav\tascript:alert("XSS");')).toBe(false);
+      expect(isSafeURL('   javascript:alert("XSS");')).toBe(false);
+      expect(isSafeURL('ftp://192.168.1.1')).toBe(false);
+      expect(isSafeURL('file:///')).toBe(false);
+      expect(isSafeURL('file:///etc/hosts')).toBe(false);
+    });
+
+    it('returns false for encoded javascript links', () => {
+      expect(
+        isSafeURL(
+          '&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          'javascript:alert('XSS')',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          '&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29',
+        ),
+      ).toBe(false);
+      expect(
+        isSafeURL(
+          '\\u006A\\u0061\\u0076\\u0061\\u0073\\u0063\\u0072\\u0069\\u0070\\u0074\\u003A\\u0061\\u006C\\u0065\\u0072\\u0074\\u0028\\u0027\\u0058\\u0053\\u0053\\u0027\\u0029',
+        ),
+      ).toBe(false);
+    });
+  });
+});

ee/spec/javascripts/vue_shared/components/safe_link_spec.js

+import SafeLink from 'ee/vue_shared/components/safe_link.vue';
+import { mountComponentWithSlots } from 'spec/helpers/vue_mount_component_helper';
+import { TEST_HOST } from 'spec/test_constants';
+import Vue from 'vue';
+
+describe('SafeLink', () => {
+  const Component = Vue.extend(SafeLink);
+  const httpLink = `${TEST_HOST}/safe_link.html`;
+  // eslint-disable-next-line no-script-url
+  const javascriptLink = 'javascript:alert("jay")';
+  const linkText = 'Link Text';
+
+  const linkProps = {
+    hreflang: 'XR',
+    rel: 'alternate',
+    type: 'text/html',
+    target: '_blank',
+    media: 'all',
+  };
+
+  let vm;
+
+  describe('valid link', () => {
+    let props;
+
+    beforeEach(() => {
+      props = { href: httpLink, ...linkProps };
+      vm = mountComponentWithSlots(Component, { props, slots: { default: [linkText] } });
+    });
+
+    it('renders a link element', () => {
+      expect(vm.$el.tagName).toEqual('A');
+    });
+
+    it('renders link specific attributes', () => {
+      expect(vm.$el.getAttribute('href')).toEqual(httpLink);
+      Object.keys(linkProps).forEach(key => {
+        expect(vm.$el.getAttribute(key)).toEqual(linkProps[key]);
+      });
+    });
+
+    it('renders the inner text as provided', () => {
+      expect(vm.$el.innerText).toEqual(linkText);
+    });
+  });
+
+  describe('invalid link', () => {
+    let props;
+
+    beforeEach(() => {
+      props = { href: javascriptLink, ...linkProps };
+      vm = mountComponentWithSlots(Component, { props, slots: { default: [linkText] } });
+    });
+
+    it('renders a span element', () => {
+      expect(vm.$el.tagName).toEqual('SPAN');
+    });
+
+    it('renders without link specific attributes', () => {
+      expect(vm.$el.getAttribute('href')).toEqual(null);
+      Object.keys(linkProps).forEach(key => {
+        expect(vm.$el.getAttribute(key)).toEqual(null);
+      });
+    });
+
+    it('renders the inner text as provided', () => {
+      expect(vm.$el.innerText).toEqual(linkText);
+    });
+  });
+});

ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js

@@ -2,6 +2,8 @@ import Vue from 'vue';
 import component from 'ee/vue_shared/security_reports/components/modal.vue';
 import createState from 'ee/vue_shared/security_reports/store/state';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
+import { trimText } from 'spec/helpers/vue_component_helper';
+import { TEST_HOST } from 'spec/test_constants';
 
 describe('Security Reports modal', () => {
   const Component = Vue.extend(component);
@@ -150,7 +152,7 @@ describe('Security Reports modal', () => {
         props.modal.data.solution.value =
           'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8';
         props.modal.data.file.value = 'Gemfile.lock';
-        props.modal.data.file.url = 'path/Gemfile.lock';
+        props.modal.data.file.url = `${TEST_HOST}/path/Gemfile.lock`;
         vm = mountComponent(Component, props);
       });
 
@@ -163,7 +165,7 @@ describe('Security Reports modal', () => {
 
       it('renders link fields with link', () => {
         expect(vm.$el.querySelector('.js-link-file').getAttribute('href')).toEqual(
-          'path/Gemfile.lock',
+          `${TEST_HOST}/path/Gemfile.lock`,
         );
       });
 
@@ -206,4 +208,63 @@ describe('Security Reports modal', () => {
       expect(vm.$el.classList.contains('modal-hide-footer')).toBeTruthy();
     });
   });
+
+  describe('does not render XSS links', () => {
+    // eslint-disable-next-line no-script-url
+    const badUrl = 'javascript:alert("")';
+
+    beforeEach(() => {
+      const props = {
+        modal: createState().modal,
+      };
+
+      props.modal.data.file.value = 'badFile.lock';
+      props.modal.data.file.url = badUrl;
+      props.modal.data.links.value = [
+        {
+          url: badUrl,
+        },
+      ];
+      props.modal.data.identifiers.value = [
+        {
+          type: 'CVE',
+          name: 'BAD_URL',
+          url: badUrl,
+        },
+      ];
+      props.modal.data.instances.value = [
+        {
+          param: 'X-Content-Type-Options',
+          method: 'GET',
+          uri: badUrl,
+        },
+      ];
+
+      vm = mountComponent(Component, props);
+    });
+
+    it('for the link field', () => {
+      const linkEl = vm.$el.querySelector('.js-link-links');
+      expect(linkEl.tagName).not.toBe('A');
+      expect(trimText(linkEl.textContent)).toBe(badUrl);
+    });
+
+    it('for the identifiers field', () => {
+      const linkEl = vm.$el.querySelector('.js-link-identifiers');
+      expect(linkEl.tagName).not.toBe('A');
+      expect(trimText(linkEl.textContent)).toBe('BAD_URL');
+    });
+
+    it('for the file field', () => {
+      const linkEl = vm.$el.querySelector('.js-link-file');
+      expect(linkEl.tagName).not.toBe('A');
+      expect(trimText(linkEl.textContent)).toBe('badFile.lock');
+    });
+
+    it('for the instances field', () => {
+      const linkEl = vm.$el.querySelector('.report-block-list-issue-description-link .break-link');
+      expect(linkEl.tagName).not.toBe('A');
+      expect(trimText(linkEl.textContent)).toBe(badUrl);
+    });
+  });
 });