From a8539ba843d387e6a3489ac8c6ebcaa47e3a1a56 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 28 Jun 2022 17:27:42 -0400 Subject: [PATCH] fix(security) Sanitize rich text before sending to backend or rendering on frontend (#5278) --- datahub-web-react/package.json | 2 ++ .../Schema/utils/useDescriptionRenderer.tsx | 10 +++++--- .../tabs/Documentation/DocumentationTab.tsx | 8 +++--- .../__tests__/DocumentationTab.test.tsx | 25 +++++++++++++++++++ .../components/DescriptionEditor.tsx | 7 ++++-- datahub-web-react/yarn.lock | 17 +++++++++++++ 6 files changed, 61 insertions(+), 8 deletions(-) diff --git a/datahub-web-react/package.json b/datahub-web-react/package.json index 64cb716a104b8d..6c2b12ba1fac56 100644 --- a/datahub-web-react/package.json +++ b/datahub-web-react/package.json @@ -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", @@ -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", diff --git a/datahub-web-react/src/app/entity/shared/tabs/Dataset/Schema/utils/useDescriptionRenderer.tsx b/datahub-web-react/src/app/entity/shared/tabs/Dataset/Schema/utils/useDescriptionRenderer.tsx index 2e4d8f2c3d1695..f6736c4e2035e3 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Dataset/Schema/utils/useDescriptionRenderer.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Dataset/Schema/utils/useDescriptionRenderer.tsx @@ -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'; @@ -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 ( updateDescription({ variables: { input: { - description: updatedDescription, + description: DOMPurify.sanitize(updatedDescription), resourceUrn: urn, subResource: record.fieldPath, subResourceType: SubResourceType.DatasetField, diff --git a/datahub-web-react/src/app/entity/shared/tabs/Documentation/DocumentationTab.tsx b/datahub-web-react/src/app/entity/shared/tabs/Documentation/DocumentationTab.tsx index ac5b43f1f804fc..a7686b3d18dd8d 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Documentation/DocumentationTab.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Documentation/DocumentationTab.tsx @@ -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'; @@ -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); @@ -51,7 +53,7 @@ export const DocumentationTab = ({ properties }: { properties?: Props }) => { ) : ( <> - {description || links.length ? ( + {sanitizedDescription || links.length ? ( <>
@@ -65,8 +67,8 @@ export const DocumentationTab = ({ properties }: { properties?: Props }) => {
- {description ? ( - + {sanitizedDescription ? ( + ) : ( No documentation added yet. )} diff --git a/datahub-web-react/src/app/entity/shared/tabs/Documentation/__tests__/DocumentationTab.test.tsx b/datahub-web-react/src/app/entity/shared/tabs/Documentation/__tests__/DocumentationTab.test.tsx index 0263cfd7e3a004..18119aa008294a 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Documentation/__tests__/DocumentationTab.test.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Documentation/__tests__/DocumentationTab.test.tsx @@ -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'; @@ -62,3 +63,27 @@ describe('SchemaDescriptionField', () => { expect(queryByText('This is a description')).not.toBeInTheDocument(); }); }); + +describe('markdown sanitization', () => { + it('should remove malicious tags like '; + const sanitizedText = DOMPurify.sanitize(text); + + expect(sanitizedText).toBe('Testing this out'); + }); + + it('should allow acceptable html', () => { + const text = 'Testing this

out

for
safety
'; + 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); + }); +}); diff --git a/datahub-web-react/src/app/entity/shared/tabs/Documentation/components/DescriptionEditor.tsx b/datahub-web-react/src/app/entity/shared/tabs/Documentation/components/DescriptionEditor.tsx index 7e91404455336f..1800320e0c5596 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Documentation/components/DescriptionEditor.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Documentation/components/DescriptionEditor.tsx @@ -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'; @@ -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, }, }, diff --git a/datahub-web-react/yarn.lock b/datahub-web-react/yarn.lock index 8b1cc6ce75d96e..72cc134fc4c231 100644 --- a/datahub-web-react/yarn.lock +++ b/datahub-web-react/yarn.lock @@ -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" @@ -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" @@ -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"