Skip to content

Commit

Permalink
fix(security) Sanitize rich text before sending to backend or renderi…
Browse files Browse the repository at this point in the history
…ng on frontend (datahub-project#5278)
  • Loading branch information
chriscollins3456 authored and alexey-kravtsov committed Jul 8, 2022
1 parent 1d39849 commit a8539ba
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 8 deletions.
2 changes: 2 additions & 0 deletions datahub-web-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"@testing-library/user-event": "^12.6.0",
"@tommoor/remove-markdown": "^0.3.2",
"@types/diff": "^5.0.0",
"@types/dompurify": "^2.3.3",
"@types/jest": "^26.0.19",
"@types/js-cookie": "^2.2.6",
"@types/node": "^12.19.9",
Expand Down Expand Up @@ -54,6 +55,7 @@
"d3-time-format": "^3.0.0",
"deepmerge": "^4.2.2",
"diff": "^5.0.0",
"dompurify": "^2.3.8",
"dotenv": "^8.2.0",
"faker": "5.5.3",
"find-webpack": "2.2.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import DOMPurify from 'dompurify';
import { EditableSchemaMetadata, SchemaField, SubResourceType } from '../../../../../../../types.generated';
import DescriptionField from '../../../../../dataset/profile/schema/components/SchemaDescriptionField';
import { pathMatchesNewPath } from '../../../../../dataset/profile/schema/utils/utils';
Expand All @@ -14,17 +15,20 @@ export default function useDescriptionRenderer(editableSchemaMetadata: EditableS
const relevantEditableFieldInfo = editableSchemaMetadata?.editableSchemaFieldInfo.find(
(candidateEditableFieldInfo) => pathMatchesNewPath(candidateEditableFieldInfo.fieldPath, record.fieldPath),
);
const displayedDescription = relevantEditableFieldInfo?.description || description;
const sanitizedDescription = DOMPurify.sanitize(displayedDescription);
const original = record.description ? DOMPurify.sanitize(record.description) : undefined;

return (
<DescriptionField
description={relevantEditableFieldInfo?.description || description}
original={record.description}
description={sanitizedDescription}
original={original}
isEdited={!!relevantEditableFieldInfo?.description}
onUpdate={(updatedDescription) =>
updateDescription({
variables: {
input: {
description: updatedDescription,
description: DOMPurify.sanitize(updatedDescription),
resourceUrn: urn,
subResource: record.fieldPath,
subResourceType: SubResourceType.DatasetField,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import styled from 'styled-components';
import { Button, Divider, Typography } from 'antd';
import { EditOutlined } from '@ant-design/icons';
import MDEditor from '@uiw/react-md-editor';
import DOMPurify from 'dompurify';

import TabToolbar from '../../components/styled/TabToolbar';
import { AddLinkModal } from '../../components/styled/AddLinkModal';
Expand All @@ -32,6 +33,7 @@ export const DocumentationTab = ({ properties }: { properties?: Props }) => {
const { urn, entityData } = useEntityData();
const refetch = useRefetch();
const description = entityData?.editableProperties?.description || entityData?.properties?.description || '';
const sanitizedDescription = DOMPurify.sanitize(description);
const links = entityData?.institutionalMemory?.elements || [];
const localStorageDictionary = localStorage.getItem(EDITED_DESCRIPTIONS_CACHE_NAME);

Expand All @@ -51,7 +53,7 @@ export const DocumentationTab = ({ properties }: { properties?: Props }) => {
</>
) : (
<>
{description || links.length ? (
{sanitizedDescription || links.length ? (
<>
<TabToolbar>
<div>
Expand All @@ -65,8 +67,8 @@ export const DocumentationTab = ({ properties }: { properties?: Props }) => {
</div>
</TabToolbar>
<DocumentationContainer>
{description ? (
<MDEditor.Markdown style={{ fontWeight: 400 }} source={description} />
{sanitizedDescription ? (
<MDEditor.Markdown style={{ fontWeight: 400 }} source={sanitizedDescription} />
) : (
<Typography.Text type="secondary">No documentation added yet.</Typography.Text>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MockedProvider } from '@apollo/client/testing';
import { render } from '@testing-library/react';
import DOMPurify from 'dompurify';
import React from 'react';
import { mocks } from '../../../../../../Mocks';
import { EntityType } from '../../../../../../types.generated';
Expand Down Expand Up @@ -62,3 +63,27 @@ describe('SchemaDescriptionField', () => {
expect(queryByText('This is a description')).not.toBeInTheDocument();
});
});

describe('markdown sanitization', () => {
it('should remove malicious tags like <script> from text', () => {
const text = 'Testing this out<script>console.log("testing")</script>';
const sanitizedText = DOMPurify.sanitize(text);

expect(sanitizedText).toBe('Testing this out');
});

it('should allow acceptable html', () => {
const text = '<strong>Testing</strong> this <p>out</p> <span>for</span> <div>safety</div>';
const sanitizedText = DOMPurify.sanitize(text);

expect(sanitizedText).toBe(text);
});

it('should allow acceptable markdown', () => {
const text =
'~~Testing~~ **this** *out* \n\n> for\n\n- safety\n\n1. ordered list\n\n[ test link](https://www.google.com/)\n';
const sanitizedText = DOMPurify.sanitize(text);

expect(sanitizedText).toBe(text);
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useState, useEffect } from 'react';
import { message, Button } from 'antd';
import { CheckOutlined } from '@ant-design/icons';
import DOMPurify from 'dompurify';

import analytics, { EventType, EntityActionType } from '../../../../../analytics';

Expand Down Expand Up @@ -31,16 +32,18 @@ export const DescriptionEditor = ({ onComplete }: { onComplete?: () => void }) =
const [cancelModalVisible, setCancelModalVisible] = useState(false);

const updateDescriptionLegacy = () => {
const sanitizedDescription = DOMPurify.sanitize(updatedDescription);
return updateEntity?.({
variables: { urn: mutationUrn, input: { editableProperties: { description: updatedDescription || '' } } },
variables: { urn: mutationUrn, input: { editableProperties: { description: sanitizedDescription || '' } } },
});
};

const updateDescription = () => {
const sanitizedDescription = DOMPurify.sanitize(updatedDescription);
return updateDescriptionMutation({
variables: {
input: {
description: updatedDescription,
description: sanitizedDescription,
resourceUrn: mutationUrn,
},
},
Expand Down
17 changes: 17 additions & 0 deletions datahub-web-react/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2826,6 +2826,13 @@
resolved "https://registry.yarnpkg.com/@types/diff/-/diff-5.0.0.tgz#eb71e94feae62548282c4889308a3dfb57e36020"
integrity sha512-jrm2K65CokCCX4NmowtA+MfXyuprZC13jbRuwprs6/04z/EcFg/MCwYdsHn+zgV4CQBiATiI7AEq7y1sZCtWKA==

"@types/dompurify@^2.3.3":
version "2.3.3"
resolved "https://registry.yarnpkg.com/@types/dompurify/-/dompurify-2.3.3.tgz#c24c92f698f77ed9cc9d9fa7888f90cf2bfaa23f"
integrity sha512-nnVQSgRVuZ/843oAfhA25eRSNzUFcBPk/LOiw5gm8mD9/X7CNcbRkQu/OsjCewO8+VIYfPxUnXvPEVGenw14+w==
dependencies:
"@types/trusted-types" "*"

"@types/eslint@^7.2.6":
version "7.2.11"
resolved "https://registry.yarnpkg.com/@types/eslint/-/eslint-7.2.11.tgz#180b58f5bb7d7376e39d22496e2b08901aa52fd2"
Expand Down Expand Up @@ -3131,6 +3138,11 @@
dependencies:
"@types/jest" "*"

"@types/trusted-types@*":
version "2.0.2"
resolved "https://registry.yarnpkg.com/@types/trusted-types/-/trusted-types-2.0.2.tgz#fc25ad9943bcac11cceb8168db4f275e0e72e756"
integrity sha512-F5DIZ36YVLE+PN+Zwws4kJogq47hNgX3Nx6WyDJ3kcplxyke3XIzB8uK5n/Lpm1HBsbGzd6nmGehL8cPekP+Tg==

"@types/uglify-js@*":
version "3.13.0"
resolved "https://registry.yarnpkg.com/@types/uglify-js/-/uglify-js-3.13.0.tgz#1cad8df1fb0b143c5aba08de5712ea9d1ff71124"
Expand Down Expand Up @@ -6910,6 +6922,11 @@ domhandler@^2.3.0:
dependencies:
domelementtype "1"

dompurify@^2.3.8:
version "2.3.8"
resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.8.tgz#224fe9ae57d7ebd9a1ae1ac18c1c1ca3f532226f"
integrity sha512-eVhaWoVibIzqdGYjwsBWodIQIaXFSB+cKDf4cfxLMsK0xiud6SE+/WCVx/Xw/UwQsa4cS3T2eITcdtmTg2UKcw==

domutils@^1.5.1, domutils@^1.7.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a"
Expand Down

0 comments on commit a8539ba

Please sign in to comment.