Merge branch 'security-2690-fix-tooltip' into 'master'

[master] Escapes job name used in tooltips

See merge request gitlab/gitlabhq!2427

app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue

@@ -1,5 +1,6 @@
 <script>
 import $ from 'jquery';
+import _ from 'underscore';
 import JobNameComponent from './job_name_component.vue';
 import JobComponent from './job_component.vue';
 import tooltip from '../../../vue_shared/directives/tooltip';
@@ -46,7 +47,7 @@ export default {
 
   computed: {
     tooltipText() {
-      return `${this.job.name} - ${this.job.status.label}`;
+      return _.escape(`${this.job.name} - ${this.job.status.label}`);
     },
   },
 

app/assets/javascripts/pipelines/components/graph/graph_component.vue

@@ -1,4 +1,5 @@
 <script>
+import _ from 'underscore';
 import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
 import StageColumnComponent from './stage_column_component.vue';
 
@@ -26,7 +27,8 @@ export default {
 
   methods: {
     capitalizeStageName(name) {
-      return name.charAt(0).toUpperCase() + name.slice(1);
+      const escapedName = _.escape(name);
+      return escapedName.charAt(0).toUpperCase() + escapedName.slice(1);
     },
 
     isFirstColumn(index) {

app/assets/javascripts/pipelines/components/graph/job_component.vue

@@ -1,4 +1,5 @@
 <script>
+import _ from 'underscore';
 import ActionComponent from './action_component.vue';
 import JobNameComponent from './job_name_component.vue';
 import tooltip from '../../../vue_shared/directives/tooltip';
@@ -61,7 +62,7 @@ export default {
       const textBuilder = [];
 
       if (this.job.name) {
-        textBuilder.push(this.job.name);
+        textBuilder.push(_.escape(this.job.name));
       }
 
       if (this.job.name && this.status.tooltip) {
@@ -69,7 +70,7 @@ export default {
       }
 
       if (this.status.tooltip) {
-        textBuilder.push(`${this.job.status.tooltip}`);
+        textBuilder.push(this.job.status.tooltip);
       }
 
       return textBuilder.join(' ');

app/assets/javascripts/pipelines/components/graph/stage_column_component.vue

@@ -1,4 +1,5 @@
 <script>
+import _ from 'underscore';
 import JobComponent from './job_component.vue';
 import DropdownJobComponent from './dropdown_job_component.vue';
 
@@ -37,7 +38,7 @@ export default {
     },
 
     jobId(job) {
-      return `ci-badge-${job.name}`;
+      return `ci-badge-${_.escape(job.name)}`;
     },
 
     buildConnnectorClass(index) {

app/views/projects/jobs/_sidebar.html.haml

@@ -86,7 +86,7 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            - tooltip = build.tooltip_message
+            - tooltip = sanitize(build.tooltip_message)
             = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }

spec/features/projects/jobs_spec.rb

@@ -135,6 +135,20 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
       end
     end
 
+    context 'sidebar' do
+      let(:job) { create(:ci_build, :success, :trace_live, pipeline: pipeline, name: '<img src=x onerror=alert(document.domain)>') }
+
+      before do
+        visit project_job_path(project, job)
+      end
+
+      it 'renders escaped tooltip name' do
+        page.within('aside.right-sidebar') do
+          expect(find('.active.build-job a')['data-title']).to eq('<img src="x"> - passed')
+        end
+      end
+    end
+
     context 'when job is not running', :js do
       let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
 

spec/javascripts/fixtures/graph.html.haml

@@ -1 +0,0 @@
-#js-pipeline-graph-vue{ data: { endpoint: "foo" } }

spec/javascripts/pipelines/graph/dropdown_job_component_spec.js

@@ -0,0 +1,93 @@
+import Vue from 'vue';
+import component from '~/pipelines/components/graph/dropdown_job_component.vue';
+import mountComponent from 'spec/helpers/vue_mount_component_helper';
+
+describe('dropdown job component', () => {
+  const Component = Vue.extend(component);
+  let vm;
+
+  const mock = {
+    jobs: [
+      {
+        id: 4256,
+        name: '<img src=x onerror=alert(document.domain)>',
+        status: {
+          icon: 'icon_status_success',
+          text: 'passed',
+          label: 'passed',
+          tooltip: 'passed',
+          group: 'success',
+          details_path: '/root/ci-mock/builds/4256',
+          has_details: true,
+          action: {
+            icon: 'retry',
+            title: 'Retry',
+            path: '/root/ci-mock/builds/4256/retry',
+            method: 'post',
+          },
+        },
+      },
+      {
+        id: 4299,
+        name: 'test',
+        status: {
+          icon: 'icon_status_success',
+          text: 'passed',
+          label: 'passed',
+          tooltip: 'passed',
+          group: 'success',
+          details_path: '/root/ci-mock/builds/4299',
+          has_details: true,
+          action: {
+            icon: 'retry',
+            title: 'Retry',
+            path: '/root/ci-mock/builds/4299/retry',
+            method: 'post',
+          },
+        },
+      },
+    ],
+    name: 'rspec:linux',
+    size: 2,
+    status: {
+      icon: 'icon_status_success',
+      text: 'passed',
+      label: 'passed',
+      tooltip: 'passed',
+      group: 'success',
+      details_path: '/root/ci-mock/builds/4256',
+      has_details: true,
+      action: {
+        icon: 'retry',
+        title: 'Retry',
+        path: '/root/ci-mock/builds/4256/retry',
+        method: 'post',
+      },
+    },
+  };
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  beforeEach(() => {
+    vm = mountComponent(Component, { job: mock });
+  });
+
+  it('renders button with job name and size', () => {
+    expect(vm.$el.querySelector('button').textContent).toContain(mock.name);
+    expect(vm.$el.querySelector('button').textContent).toContain(mock.size);
+  });
+
+  it('renders dropdown with jobs', () => {
+    expect(vm.$el.querySelectorAll('.scrollable-menu>ul>li').length).toEqual(mock.jobs.length);
+  });
+
+  it('escapes tooltip title', () => {
+    expect(
+      vm.$el.querySelector('.js-pipeline-graph-job-link').getAttribute('data-original-title'),
+    ).toEqual(
+      '&lt;img src=x onerror=alert(document.domain)&gt; - passed',
+    );
+  });
+});

spec/javascripts/pipelines/graph/graph_component_spec.js

@@ -1,37 +1,33 @@
 import Vue from 'vue';
+import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import graphComponent from '~/pipelines/components/graph/graph_component.vue';
 import graphJSON from './mock_data';
 
 describe('graph component', () => {
-  preloadFixtures('static/graph.html.raw');
+  const GraphComponent = Vue.extend(graphComponent);
+  let component;
 
-  let GraphComponent;
-
-  beforeEach(() => {
-    loadFixtures('static/graph.html.raw');
-    GraphComponent = Vue.extend(graphComponent);
+  afterEach(() => {
+    component.$destroy();
   });
 
   describe('while is loading', () => {
     it('should render a loading icon', () => {
-      const component = new GraphComponent({
-        propsData: {
+      component = mountComponent(GraphComponent, {
         isLoading: true,
         pipeline: {},
-        },
-      }).$mount('#js-pipeline-graph-vue');
+      });
+
       expect(component.$el.querySelector('.loading-icon')).toBeDefined();
     });
   });
 
   describe('with data', () => {
     it('should render the graph', () => {
-      const component = new GraphComponent({
-        propsData: {
+      component = mountComponent(GraphComponent, {
         isLoading: false,
         pipeline: graphJSON,
-        },
-      }).$mount('#js-pipeline-graph-vue');
+      });
 
       expect(component.$el.classList.contains('js-pipeline-graph')).toEqual(true);
 
@@ -52,4 +48,15 @@ describe('graph component', () => {
       expect(component.$el.querySelector('.stage-column-list')).toBeDefined();
     });
   });
+
+  describe('capitalizeStageName', () => {
+    it('capitalizes and escapes stage name', () => {
+      component = mountComponent(GraphComponent, {
+        isLoading: false,
+        pipeline: graphJSON,
+      });
+
+      expect(component.$el.querySelector('.stage-column:nth-child(2) .stage-name').textContent.trim()).toEqual('Deploy <img src=x onerror=alert(document.domain)>');
+    });
+  });
 });

spec/javascripts/pipelines/graph/job_component_spec.js

@@ -3,7 +3,7 @@ import jobComponent from '~/pipelines/components/graph/job_component.vue';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
 
 describe('pipeline graph job component', () => {
-  let JobComponent;
+  const JobComponent = Vue.extend(jobComponent);
   let component;
 
   const mockJob = {
@@ -26,10 +26,6 @@ describe('pipeline graph job component', () => {
     },
   };
 
-  beforeEach(() => {
-    JobComponent = Vue.extend(jobComponent);
-  });
-
   afterEach(() => {
     component.$destroy();
   });
@@ -165,4 +161,24 @@ describe('pipeline graph job component', () => {
       expect(component.$el.querySelector(tooltipBoundary)).toBeNull();
     });
   });
+
+  describe('tooltipText', () => {
+    it('escapes job name', () => {
+      component = mountComponent(JobComponent, {
+        job: {
+          id: 4259,
+          name: '<img src=x onerror=alert(document.domain)>',
+          status: {
+            icon: 'icon_status_success',
+            label: 'success',
+            tooltip: 'failed',
+          },
+        },
+      });
+
+      expect(
+        component.$el.querySelector('.js-job-component-tooltip').getAttribute('data-original-title'),
+      ).toEqual('&lt;img src=x onerror=alert(document.domain)&gt; - failed');
+    });
+  });
 });

spec/javascripts/pipelines/graph/mock_data.js

@@ -91,7 +91,7 @@ export default {
         dropdown_path: '/root/ci-mock/pipelines/123/stage.json?stage=test',
       },
       {
-        name: 'deploy',
+        name: 'deploy <img src=x onerror=alert(document.domain)>',
         title: 'deploy: passed',
         groups: [
           {

spec/javascripts/pipelines/graph/stage_column_component_spec.js

@@ -1,8 +1,11 @@
 import Vue from 'vue';
 import stageColumnComponent from '~/pipelines/components/graph/stage_column_component.vue';
+import mountComponent from 'spec/helpers/vue_mount_component_helper';
 
 describe('stage column component', () => {
   let component;
+  const StageColumnComponent = Vue.extend(stageColumnComponent);
+
   const mockJob = {
     id: 4250,
     name: 'test',
@@ -22,7 +25,6 @@ describe('stage column component', () => {
   };
 
   beforeEach(() => {
-    const StageColumnComponent = Vue.extend(stageColumnComponent);
 
     const mockJobs = [];
     for (let i = 0; i < 3; i += 1) {
@@ -31,12 +33,10 @@ describe('stage column component', () => {
       mockJobs.push(mockedJob);
     }
 
-    component = new StageColumnComponent({
-      propsData: {
+    component = mountComponent(StageColumnComponent, {
       title: 'foo',
       jobs: mockJobs,
-      },
-    }).$mount();
+    });
   });
 
   it('should render provided title', () => {
@@ -46,4 +46,27 @@ describe('stage column component', () => {
   it('should render the provided jobs', () => {
     expect(component.$el.querySelectorAll('.builds-container > ul > li').length).toEqual(3);
   });
+
+  describe('jobId', () => {
+    it('escapes job name', () => {
+      component = mountComponent(StageColumnComponent, {
+        jobs: [
+          {
+            id: 4259,
+            name: '<img src=x onerror=alert(document.domain)>',
+            status: {
+              icon: 'icon_status_success',
+              label: 'success',
+              tooltip: '<img src=x onerror=alert(document.domain)>',
+            },
+          },
+        ],
+        title: 'test',
+      });
+
+      expect(
+        component.$el.querySelector('.builds-container li').getAttribute('id'),
+      ).toEqual('ci-badge-&lt;img src=x onerror=alert(document.domain)&gt;');
+    });
+  });
 });