Skip to content

Commit cfbb3ef

Browse files
author
Karol Tatała
authored
Merge pull request #192 from Wikia/IW-4876
IW-4876 | Fix XSS in notifacations
2 parents 6ecae33 + 2d52f47 commit cfbb3ef

File tree

10 files changed

+337
-247
lines changed

10 files changed

+337
-247
lines changed

‎docs/build/1.7cd9ad06.js‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/build/1.8142c86e.js‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎docs/build/bundle.4e3e350e.js‎

Lines changed: 0 additions & 79 deletions
This file was deleted.

‎docs/build/bundle.a2f5ff66.js‎

Lines changed: 79 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/index.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<!DOCTYPE html><html><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"><meta http-equiv="X-UA-Compatible" content="ie=edge"><title>react-common</title><link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Rubik:300,300i,400,400i,500,500i,700,700i,900,900i&display=swap"></head><body><div id="rsg-root"></div><div id="app"></div><script src="build/bundle.4e3e350e.js"></script></body></html>
1+
<!DOCTYPE html><html><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width, initial-scale=1.0"><meta http-equiv="X-UA-Compatible" content="ie=edge"><title>react-common</title><link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Rubik:300,300i,400,400i,500,500i,700,700i,900,900i&display=swap"></head><body><div id="rsg-root"></div><div id="app"></div><script src="build/bundle.a2f5ff66.js"></script></body></html>

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
"@babel/preset-react": "^7.8.3",
6464
"@rollup/plugin-commonjs": "^11.0.2",
6565
"@rollup/plugin-node-resolve": "^7.1.1",
66+
"@testing-library/react": "^11.2.2",
6667
"babel-core": "^7.0.0-bridge.0",
6768
"babel-eslint": "^10.1.0",
6869
"babel-jest": "^25.1.0",

‎source/components/GlobalNavigation/components/Notifications/CardText.js‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React, { useContext } from 'react';
22
import PropTypes from 'prop-types';
3-
import { useTranslation, Trans } from 'react-i18next';
3+
import { useTranslation } from 'react-i18next';
44
import get from 'lodash/get';
55

66
import {
@@ -18,10 +18,23 @@ function bold(text) {
1818
return `<b>${text}</b>`;
1919
}
2020

21+
function escapeHtml(unsafe) {
22+
if (!unsafe) {
23+
return unsafe;
24+
}
25+
26+
return unsafe
27+
.replace(/&/g, '&amp;')
28+
.replace(/</g, '&lt;')
29+
.replace(/>/g, '&gt;')
30+
.replace(/"/g, '&quot;')
31+
.replace(/'/g, '&#039;');
32+
}
33+
2134
const getReplyMessageBody = (translateFunc, { title, totalUniqueActors, latestActors, postTitleMarkup }) => {
2235
const hasTwoUsers = totalUniqueActors === 2;
2336
const hasThreeOrMoreUsers = totalUniqueActors > 2;
24-
const firstReplierName = get(latestActors, '[0].name');
37+
const firstReplierName = escapeHtml(get(latestActors, '[0].name'));
2538

2639
if (title) {
2740
if (hasThreeOrMoreUsers) {
@@ -35,7 +48,7 @@ const getReplyMessageBody = (translateFunc, { title, totalUniqueActors, latestAc
3548
if (hasTwoUsers) {
3649
return translateFunc('notifications-replied-by-two-users-with-title', {
3750
firstUser: firstReplierName,
38-
secondUser: get(latestActors, '[1].name'),
51+
secondUser: escapeHtml(get(latestActors, '[1].name')),
3952
postTitle: postTitleMarkup,
4053
});
4154
}
@@ -55,7 +68,7 @@ const getReplyMessageBody = (translateFunc, { title, totalUniqueActors, latestAc
5568
if (hasTwoUsers) {
5669
return translateFunc('notifications-replied-by-two-users-no-title', {
5770
firstUser: firstReplierName,
58-
secondUser: get(latestActors, '[1].name'),
71+
secondUser: escapeHtml(get(latestActors, '[1].name')),
5972
});
6073
}
6174

@@ -116,16 +129,16 @@ const getReplyUpvoteMessageBody = (translateFunc, { title, totalUniqueActors, po
116129

117130
const getPostAtMentionMessageBody = (translateFunc, { postTitleMarkup, latestActors }) => translateFunc('notifications-reply-at-mention', {
118131
postTitle: postTitleMarkup,
119-
mentioner: get(latestActors, '[0].name'),
132+
mentioner: escapeHtml(get(latestActors, '[0].name')),
120133
});
121134

122135
const getThreadAtMentionMessageBody = (translateFunc, { postTitleMarkup, latestActors }) => translateFunc('notifications-post-at-mention', {
123136
postTitle: postTitleMarkup,
124-
mentioner: get(latestActors, '[0].name'),
137+
mentioner: escapeHtml(get(latestActors, '[0].name')),
125138
});
126139

127140
function getArticleCommentNotificationUsername(t, latestActors) {
128-
return get(latestActors, '[0].name') ?? t('notifications-anon-user');
141+
return escapeHtml(get(latestActors, '[0].name')) ?? t('notifications-anon-user');
129142
}
130143

131144
function getArticleCommentReplyMessageBody(t, { userData, refersToAuthorId, latestActors, title }) {
@@ -150,7 +163,8 @@ function getArticleCommentReplyAtMentionMessageBody(t, { latestActors, title })
150163
}
151164

152165
const getText = (translateFunc, model, userData) => {
153-
const { type, snippet, title, totalUniqueActors, latestActors, refersToAuthorId } = model;
166+
const { type, snippet, title: dangerousTitle, totalUniqueActors, latestActors, refersToAuthorId } = model;
167+
const title = escapeHtml(dangerousTitle);
154168
const postTitleMarkup = `<b>${title}</b>`;
155169

156170
if (isAnnouncement(type)) {
@@ -197,7 +211,8 @@ const CardText = ({ model }) => {
197211
const userData = useUserData();
198212
const text = getText(t, model, userData);
199213

200-
return <p className="wds-notification-card__text"><Trans t={t}>{text}</Trans></p>;
214+
// eslint-disable-next-line react/no-danger
215+
return <p className="wds-notification-card__text" dangerouslySetInnerHTML={{ __html: text }} />;
201216
};
202217

203218
CardText.propTypes = {

‎source/components/GlobalNavigation/components/Notifications/CardText.spec.js‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
2-
import ShallowRenderer from 'react-test-renderer/shallow';
32
import merge from 'lodash/merge';
3+
import { render } from '@testing-library/react';
44

55
import { notificationTypes } from '../../models/notificationTypes';
66
import { useUserData } from '../../context/UserContext';
@@ -10,7 +10,7 @@ import CardText from './CardText';
1010
const actorsMock = [
1111
{
1212
id: '12345',
13-
name: 'ATOB',
13+
name: 'ATO<b>B</b>',
1414
avatarUrl: 'https://static.wikia.nocookie.net/458a839b-8f84-42c2-b9d1-cf8c5a21bd75',
1515
profileUrl: 'http://localhost:6060/wiki/User:ATOB',
1616
src: 'https://static.wikia.nocookie.net/458a839b-8f84-42c2-b9d1-cf8c5a21bd75',
@@ -35,6 +35,11 @@ jest.mock('../../context/UserContext', () => ({
3535
useUserData: jest.fn(),
3636
}));
3737

38+
jest.mock('react-i18next', () => ({
39+
...jest.requireActual('react-i18next'),
40+
useTranslation: () => [jest.fn((key, params) => `${key}${params ? JSON.stringify(params) : ''}`)],
41+
}));
42+
3843
const defaultProps = {
3944
track: () => null,
4045
model: {
@@ -45,20 +50,19 @@ const defaultProps = {
4550
latestEventUri: 'http://xkxd.wikia.com/d/p/3100000000000001096/r/3086787452863005635',
4651
snippet: 'some snippet',
4752
timestamp: 1550663179,
48-
title: 'MOAR LIKES',
53+
title: 'MOAR <b>LIKES</b>',
4954
type: notificationTypes.discussionReply,
5055
totalUniqueActors: actorsMock.length,
5156
uri: 'http://xkxd.wikia.com/d/p/3100000000000001096',
5257
},
5358
};
5459

5560
function renderComponent(props) {
56-
const renderer = new ShallowRenderer();
5761
const computedProps = merge({}, defaultProps, props);
5862

59-
renderer.render(<CardText {...computedProps} />);
63+
const { container } = render(<CardText {...computedProps} />);
6064

61-
return renderer.getRenderOutput();
65+
return container.firstChild;
6266
}
6367

6468
test('CardText renders correctly with default props', () => {
@@ -223,7 +227,7 @@ describe('Discussion at mentions', () => {
223227
type: notificationTypes.postAtMention,
224228
latestActors: actorsMock.slice(0, 1),
225229
totalUniqueActors: 1,
226-
title: 'Post with at mention',
230+
title: 'Post with at <b>mention</b>',
227231
},
228232
})).toMatchSnapshot();
229233
});
@@ -234,7 +238,7 @@ describe('Discussion at mentions', () => {
234238
type: notificationTypes.threadAtMention,
235239
latestActors: actorsMock.slice(0, 1),
236240
totalUniqueActors: 1,
237-
title: 'Post with at mention',
241+
title: 'Post with at <b>mention</b>',
238242
},
239243
})).toMatchSnapshot();
240244
});
@@ -250,7 +254,7 @@ describe('Article Comments', () => {
250254
type: notificationTypes.articleCommentReply,
251255
latestActors: actorsMock.slice(0, 1),
252256
totalUniqueActors: 1,
253-
title: 'Article Title',
257+
title: 'Article <b>Title</b>',
254258
refersToAuthorId: someMockAuthor.id,
255259
},
256260
})).toMatchSnapshot();
@@ -265,7 +269,7 @@ describe('Article Comments', () => {
265269
type: notificationTypes.articleCommentReply,
266270
latestActors: actorsMock.slice(0, 1),
267271
totalUniqueActors: 1,
268-
title: 'Article Title',
272+
title: 'Article <b>Title</b>',
269273
refersToAuthorId: '11111',
270274
},
271275
})).toMatchSnapshot();
@@ -283,7 +287,7 @@ describe('Article Comments at mentions', () => {
283287
type: notificationTypes.articleCommentAtMention,
284288
latestActors: actorsMock.slice(0, 1),
285289
totalUniqueActors: 1,
286-
title: 'Article Title',
290+
title: 'Article <b>Title</b>',
287291
},
288292
})).toMatchSnapshot();
289293
});
@@ -294,7 +298,7 @@ describe('Article Comments at mentions', () => {
294298
type: notificationTypes.articleCommentReplyAtMention,
295299
latestActors: actorsMock.slice(0, 1),
296300
totalUniqueActors: 1,
297-
title: 'Article Title',
301+
title: 'Article <b>Title</b>',
298302
},
299303
})).toMatchSnapshot();
300304
});
@@ -305,7 +309,7 @@ describe('Article Comments at mentions', () => {
305309
type: notificationTypes.articleCommentReplyAtMention,
306310
latestActors: [{ id: 0, name: null }],
307311
totalUniqueActors: 1,
308-
title: 'Article Title',
312+
title: 'Article <b>Title</b>',
309313
},
310314
})).toMatchSnapshot();
311315
});

0 commit comments

Comments
 (0)