Skip to content

Commit

Permalink
Mitigate XSS in element.attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
12joan committed Sep 18, 2024
1 parent 3b3c08c commit d30471c
Show file tree
Hide file tree
Showing 7 changed files with 250 additions and 86 deletions.
5 changes: 5 additions & 0 deletions .changeset/lovely-taxis-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@udecode/plate-media': patch
---

Add the `alt` attribute to `dangerouslyAllowElementAttributes`
31 changes: 31 additions & 0 deletions .changeset/olive-shrimps-joke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
'@udecode/plate-core': patch
---

Mitigate XSS in `element.attributes` by requiring all attribute names to be allowlisted in the `node.dangerouslyAllowElementAttributes` plugin configuration option.

Migration:

For each plugin that needs to support passing DOM attributes using `element.attributes`, add the list of allowed attributes to the `node.dangerouslyAllowElementAttributes` option of the plugin.

````ts
const ImagePlugin = createPlatePlugin({
key: 'image',
node: {
isElement: true,
isVoid: true,
dangerouslyAllowElementAttributes: ['alt'],
},
});

To modify existing plugins, use the `extend` method as follows:

```ts
const MyImagePlugin = ImagePlugin.extend({
node: {
dangerouslyAllowElementAttributes: ['alt'],
},
});
````
WARNING: Improper use of `dangerouslyAllowElementAttributes` WILL make your application vulnerable to cross-site scripting (XSS) or information exposure attacks. Ensure you carefully research the security implications of any attribute before adding it. For example, the `src` and `href` attributes will allow attackers to execute arbitrary code, and the `style` and `background` attributes will allow attackers to leak users' IP addresses.
31 changes: 31 additions & 0 deletions packages/core/src/lib/plugin/BasePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,37 @@ export type BasePluginNode = {
*/
type: string;

/**
* Controls which (if any) attribute names in the `attributes` property of an
* element will be passed as `nodeProps` to the {@link NodeComponent}, and
* subsequently rendered as DOM attributes.
*
* WARNING: If used improperly, this property WILL make your application
* vulnerable to cross-site scripting (XSS) or information exposure attacks.
*
* For example, if the `href` attribute is allowed and the component passes
* `nodeProps` to an `<a>` element, then attackers can direct users to open a
* document containing a malicious link element:
*
* { type: 'link', url: 'https://safesite.com/', attributes: { href:
* 'javascript:alert("xss")' }, children: [{ text: 'Click me' }], }
*
* The same is true of the `src` attribute when passed to certain HTML
* elements, such as `<iframe>`.
*
* If the `style` attribute (or another attribute that can load URLs, such as
* `background`) is allowed, then attackers can direct users to open a
* document that will send a HTTP request to an arbitrary URL. This can leak
* the victim's IP address or confirm to the attacker that the victim opened
* the document.
*
* Before allowing any attribute name, ensure that you thoroughly research and
* assess any potential risks associated with it.
*
* @default [ ]
*/
dangerouslyAllowElementAttributes?: string[];

/**
* Indicates if this plugin's nodes should be rendered as elements. Used by
* Plate for {@link NodeComponent} rendering as elements.
Expand Down
83 changes: 82 additions & 1 deletion packages/core/src/react/components/Plate.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { type Value, isBlock, setNodes } from '@udecode/slate';
import isEqual from 'lodash/isEqual';
import memoize from 'lodash/memoize';

import type { PlatePlugins } from '../plugin';
import type { PlatePlugins, PlateRenderElementProps } from '../plugin';

import { type SlatePlugins, createSlatePlugin } from '../../lib';
import { createPlateEditor, usePlateEditor } from '../editor';
Expand Down Expand Up @@ -470,4 +470,85 @@ describe('Plate', () => {
expect(mountCount).toBe(2);
});
});

describe('User-defined attributes', () => {
const ParagraphElement = ({
attributes,
children,
nodeProps,
}: PlateRenderElementProps) => (
<p {...attributes} {...nodeProps}>
{children}
</p>
);

const paragraphPlugin = (dangerouslyAllowElementAttributes: boolean) =>
createPlatePlugin({
key: 'p',
node: {
component: ParagraphElement,
dangerouslyAllowElementAttributes: dangerouslyAllowElementAttributes
? ['data-my-attribute']
: undefined,
isElement: true,
},
});

const initialValue = [
{
attributes: {
'data-my-attribute': 'hello',
'data-unpermitted-attribute': 'world',
},
children: [{ text: 'My paragraph' }],
type: 'p',
},
];

const Editor = ({
dangerouslyAllowElementAttributes,
}: {
dangerouslyAllowElementAttributes: boolean;
}) => {
const editor = usePlateEditor({
plugins: [paragraphPlugin(dangerouslyAllowElementAttributes)],
value: initialValue,
});

return (
<Plate editor={editor}>
<PlateContent />
</Plate>
);
};

it('renders no user-defined attributes by default', () => {
const { container } = render(
<Editor dangerouslyAllowElementAttributes={false} />
);

// eslint-disable-next-line testing-library/no-container
const paragraphEl = container.querySelector(
'[data-slate-node=element]'
) as HTMLElement;

expect(Object.keys(paragraphEl.dataset)).toEqual(['slateNode']);
});

it('renders allowed user-defined attributes', () => {
const { container } = render(
<Editor dangerouslyAllowElementAttributes={true} />
);

// eslint-disable-next-line testing-library/no-container
const paragraphEl = container.querySelector(
'[data-slate-node=element]'
) as HTMLElement;

expect(Object.keys(paragraphEl.dataset)).toEqual([
'slateNode',
'myAttribute',
]);
});
});
});
18 changes: 15 additions & 3 deletions packages/core/src/react/utils/getRenderNodeProps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AnyObject } from '@udecode/utils';

import { clsx } from 'clsx';
import { pick } from 'lodash';

import type { PlateEditor } from '../editor';
import type { AnyEditorPlatePlugin } from '../plugin/PlatePlugin';
Expand All @@ -10,8 +11,9 @@ import { getSlateClass } from '../../lib';
import { getEditorPlugin } from '../plugin';

/**
* Override node props with plugin props. `props.element.attributes` are passed
* as `nodeProps`. Extend the class name with the node type.
* Override node props with plugin props. Allowed properties in
* `props.element.attributes` are passed as `nodeProps`. Extend the class name
* with the node type.
*/
export const getRenderNodeProps = ({
attributes,
Expand All @@ -33,7 +35,17 @@ export const getRenderNodeProps = ({
: plugin.node.props) ?? {};
}
if (!newProps.nodeProps && attributes) {
newProps.nodeProps = attributes;
/**
* WARNING: Improper use of `dangerouslyAllowElementAttributes` WILL make
* your application vulnerable to cross-site scripting (XSS) or information
* exposure attacks.
*
* @see {@link BasePluginNode.dangerouslyAllowElementAttributes}
*/
newProps.nodeProps = pick(
attributes,
plugin.node.dangerouslyAllowElementAttributes ?? []
);
}

props = { ...props, ...newProps };
Expand Down
6 changes: 5 additions & 1 deletion packages/media/src/lib/image/BaseImagePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ export type ImageConfig = PluginConfig<
export const BaseImagePlugin = createTSlatePlugin<ImageConfig>({
key: 'img',
extendEditor: withImage,
node: { isElement: true, isVoid: true },
node: {
dangerouslyAllowElementAttributes: ['alt'],
isElement: true,
isVoid: true,
},
}).extend(({ plugin }) => ({
parsers: {
html: {
Expand Down
Loading

0 comments on commit d30471c

Please sign in to comment.