Commit d0e3bb36 authored by Alexander Turinske's avatar Alexander Turinske

Sanitize vulnerability history comment

- add sanitation to vulnerability history comments
- add additional tests for XSS of alert and iframe
- add changelog for comment sanitization
- use comment.note_url instead of comment.note to render html
- use .md class to normalize incoming CSS
- Use v-html on note_url
- Update sanitize import to come from dompurify
- we no longer use the library `sanitize-html`, but instead use
  the library `dompurify`
- use the `sanitize` method from `dompurify` and update tests
parent 8864beb2
......@@ -42,9 +42,6 @@ export default {
},
computed: {
commentNote() {
return this.comment?.note;
},
actionButtons() {
return [
{
......@@ -59,6 +56,9 @@ export default {
},
];
},
initialComment() {
return this.comment && this.comment.note;
},
},
methods: {
......@@ -135,8 +135,8 @@ export default {
<template>
<history-comment-editor
v-if="isEditingComment"
class="discussion-reply-holder m-3"
:initial-comment="commentNote"
class="discussion-reply-holder"
:initial-comment="initialComment"
:is-saving="isSavingComment"
@onSave="saveComment"
@onCancel="cancelEditingComment"
......@@ -154,7 +154,7 @@ export default {
icon-class="timeline-icon m-0"
class="m-3"
>
<div v-html="comment.note"></div>
<div class="md" v-html="comment.note_html"></div>
<template #right-content>
<gl-button
......
<script>
import { sanitize } from 'dompurify';
import { GlFormTextarea, GlButton, GlLoadingIcon } from '@gitlab/ui';
export default {
......@@ -21,10 +22,10 @@ export default {
},
computed: {
isSaveButtonDisabled() {
return this.isSaving || !this.trimmedComment.length;
return this.isSaving || !this.sanitizedComment.length;
},
trimmedComment() {
return this.comment.trim();
sanitizedComment() {
return sanitize(this.comment.trim());
},
},
};
......@@ -43,7 +44,7 @@ export default {
ref="saveButton"
variant="success"
:disabled="isSaveButtonDisabled"
@click="$emit('onSave', trimmedComment)"
@click="$emit('onSave', sanitizedComment)"
>
<gl-loading-icon v-if="isSaving" class="mr-1" />
{{ __('Save comment') }}
......
---
title: Sanitize vulnerability history comment
merge_request:
author:
type: security
......@@ -23,12 +23,14 @@ describe('Event Item', () => {
},
};
afterEach(() => {
wrapper.destroy();
});
const slots = { default: '<p>Test</p>' };
beforeEach(() => {
mountComponent({ propsData });
mountComponent({ propsData, slots });
});
afterEach(() => {
wrapper.destroy();
});
it('passes the expected values to the note header component', () => {
......@@ -51,6 +53,10 @@ describe('Event Item', () => {
it('renders the action buttons container', () => {
expect(wrapper.find('.action-buttons')).toExist();
});
it('renders the default slot', () => {
expect(wrapper.html()).toEqual(expect.stringContaining('<p>Test</p>'));
});
});
describe('with action buttons', () => {
const propsData = {
......
......@@ -44,6 +44,28 @@ describe('History Comment Editor', () => {
expect(wrapper.emitted().onSave[0][0]).toBe(comment);
});
it('sanitizes a new comment to protect against an XSS attack', () => {
createWrapper();
const comment = '"><img src=x onerror=alert(document.domain)>';
const sanitizedComment = '"&gt;<img src="x">';
textarea().vm.$emit('input', comment);
saveButton().vm.$emit('click');
expect(wrapper.emitted().onSave).toHaveLength(1);
expect(wrapper.emitted().onSave[0][0]).toBe(sanitizedComment);
});
it('sanitizes a new comment to protect against an XSS attack using an iframe', () => {
createWrapper();
const comment = `"><iframe src='hxxps://nefarious.com'>'`;
const sanitizedComment = `"&gt;`;
textarea().vm.$emit('input', comment);
saveButton().vm.$emit('click');
expect(wrapper.emitted().onSave).toHaveLength(1);
expect(wrapper.emitted().onSave[0][0]).toBe(sanitizedComment);
});
it('emits the cancel event when the cancel button is clicked', () => {
createWrapper();
cancelButton().vm.$emit('click');
......
......@@ -24,6 +24,7 @@ describe('History Comment', () => {
const comment = {
id: 'id',
note: 'note',
note_html: '<p>note</p>',
author: {},
updated_at: new Date().toISOString(),
current_user: {
......@@ -142,7 +143,7 @@ describe('History Comment', () => {
expectExistingCommentView();
expect(eventItem().props('author')).toBe(comment.author);
expect(eventItem().props('createdAt')).toBe(comment.updated_at);
expect(eventItem().element.innerHTML).toContain(comment.note);
expect(eventItem().element.innerHTML).toContain(comment.note_html);
});
it('shows the comment editor when the edit button is clicked', () => {
......@@ -160,7 +161,7 @@ describe('History Comment', () => {
})
.then(() => {
expectExistingCommentView();
expect(eventItem().element.innerHTML).toContain(comment.note);
expect(eventItem().element.innerHTML).toContain(comment.note_html);
});
});
......@@ -181,7 +182,7 @@ describe('History Comment', () => {
})
.then(() => {
expectExistingCommentView();
expect(eventItem().element.innerHTML).toContain(comment.note);
expect(eventItem().element.innerHTML).toContain(comment.note_html);
});
});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment