Skip to content

Commit 936bde3

Browse files
committed
Sanitize html props with xss vulnerability.
1 parent 000ec18 commit 936bde3

File tree

8 files changed

+187
-57
lines changed

8 files changed

+187
-57
lines changed

components/dash-core-components/package-lock.json

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

components/dash-core-components/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"maintainer": "Alex Johnson <[email protected]>",
3636
"license": "MIT",
3737
"dependencies": {
38+
"@braintree/sanitize-url": "^7.0.0",
3839
"@fortawesome/fontawesome-svg-core": "1.2.36",
3940
"@fortawesome/free-regular-svg-icons": "^5.15.4",
4041
"@fortawesome/free-solid-svg-icons": "^5.15.4",

components/dash-core-components/src/components/Link.react.js

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import PropTypes from 'prop-types';
22

3-
import React, {Component} from 'react';
4-
3+
import React, {useEffect, useMemo} from 'react';
4+
import {sanitizeUrl} from '@braintree/sanitize-url';
55
import {isNil} from 'ramda';
66

77
/*
@@ -33,15 +33,23 @@ CustomEvent.prototype = window.Event.prototype;
3333
* For links with destinations outside the current app, `html.A` is a better
3434
* component to use.
3535
*/
36-
export default class Link extends Component {
37-
constructor(props) {
38-
super(props);
39-
this.updateLocation = this.updateLocation.bind(this);
40-
}
36+
const Link = props => {
37+
const {
38+
className,
39+
style,
40+
id,
41+
href,
42+
loading_state,
43+
children,
44+
title,
45+
target,
46+
refresh,
47+
setProps,
48+
} = props;
49+
const sanitizedUrl = useMemo(() => sanitizeUrl(href), [href]);
4150

42-
updateLocation(e) {
51+
const updateLocation = e => {
4352
const hasModifiers = e.metaKey || e.shiftKey || e.altKey || e.ctrlKey;
44-
const {href, refresh, target} = this.props;
4553

4654
if (hasModifiers) {
4755
return;
@@ -52,49 +60,45 @@ export default class Link extends Component {
5260
// prevent anchor from updating location
5361
e.preventDefault();
5462
if (refresh) {
55-
window.location = href;
63+
window.location = sanitizedUrl;
5664
} else {
57-
window.history.pushState({}, '', href);
65+
window.history.pushState({}, '', sanitizedUrl);
5866
window.dispatchEvent(new CustomEvent('_dashprivate_pushstate'));
5967
}
6068
// scroll back to top
6169
window.scrollTo(0, 0);
62-
}
70+
};
6371

64-
render() {
65-
const {
66-
className,
67-
style,
68-
id,
69-
href,
70-
loading_state,
71-
children,
72-
title,
73-
target,
74-
} = this.props;
75-
/*
76-
* ideally, we would use cloneElement however
77-
* that doesn't work with dash's recursive
78-
* renderTree implementation for some reason
79-
*/
80-
return (
81-
<a
82-
data-dash-is-loading={
83-
(loading_state && loading_state.is_loading) || undefined
84-
}
85-
id={id}
86-
className={className}
87-
style={style}
88-
href={href}
89-
onClick={e => this.updateLocation(e)}
90-
title={title}
91-
target={target}
92-
>
93-
{isNil(children) ? href : children}
94-
</a>
95-
);
96-
}
97-
}
72+
useEffect(() => {
73+
if (sanitizedUrl !== href) {
74+
setProps({
75+
_dash_error: new Error(`Dangerous link detected:: ${href}`),
76+
});
77+
}
78+
}, [href, sanitizedUrl]);
79+
80+
/*
81+
* ideally, we would use cloneElement however
82+
* that doesn't work with dash's recursive
83+
* renderTree implementation for some reason
84+
*/
85+
return (
86+
<a
87+
data-dash-is-loading={
88+
(loading_state && loading_state.is_loading) || undefined
89+
}
90+
id={id}
91+
className={className}
92+
style={style}
93+
href={sanitizedUrl}
94+
onClick={updateLocation}
95+
title={title}
96+
target={target}
97+
>
98+
{isNil(children) ? sanitizedUrl : children}
99+
</a>
100+
);
101+
};
98102

99103
Link.propTypes = {
100104
/**
@@ -151,8 +155,10 @@ Link.propTypes = {
151155
*/
152156
component_name: PropTypes.string,
153157
}),
158+
setProps: PropTypes.func,
154159
};
155160

156161
Link.defaultProps = {
157162
refresh: false,
158163
};
164+
export default Link;

components/dash-html-components/package-lock.json

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

components/dash-html-components/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@
2828
"author": "Chris Parmer <[email protected]>",
2929
"maintainer": "Alex Johnson <[email protected]>",
3030
"dependencies": {
31+
"@braintree/sanitize-url": "^7.0.0",
3132
"prop-types": "^15.8.1",
3233
"ramda": "^0.29.0",
33-
"react": "^18.2.0",
34-
"react-dom": "^18.2.0"
34+
"react": ">=17",
35+
"react-dom": ">=17"
3536
},
3637
"devDependencies": {
3738
"@babel/cli": "^7.23.0",

components/dash-html-components/scripts/generate-components.js

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,27 +249,68 @@ const customDocs = {
249249
* <body>.`
250250
};
251251

252+
const customImportsForComponents = {
253+
a: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
254+
form: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
255+
iframe: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
256+
object: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
257+
embed: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
258+
button: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
259+
img: `import {sanitizeUrl} from '@braintree/sanitize-url';`,
260+
}
261+
262+
function createXSSProtection(propName) {
263+
return `
264+
const ${propName} = React.useMemo(() => props.${propName} ? sanitizeUrl(props.${propName}): undefined, [props.${propName}]);
265+
266+
if (${propName}) {
267+
extraProps.${propName} = ${propName};
268+
}
269+
270+
React.useEffect(() => {
271+
if (${propName} && ${propName} !== props.${propName}) {
272+
props.setProps({_dash_error: new Error(\`Dangerous link detected: \${props.${propName}}\`)})
273+
}
274+
}, [props.${propName}, ${propName}]);
275+
`
276+
}
277+
278+
279+
const customCodesForComponents = {
280+
a: createXSSProtection('href'),
281+
form: createXSSProtection('action'),
282+
iframe: createXSSProtection('src'),
283+
object: createXSSProtection('data'),
284+
embed: createXSSProtection('src'),
285+
button: createXSSProtection('formAction'),
286+
img: createXSSProtection('src'),
287+
}
288+
252289
function generateComponent(Component, element, attributes) {
253290
const propTypes = generatePropTypes(element, attributes);
254291

292+
const customImport = customImportsForComponents[element] || '';
255293
const customDoc = customDocs[element] ? ('\n *' + customDocs[element] + '\n *') : '';
256294

295+
const customCode = customCodesForComponents[element] || '';
296+
257297
return `
258298
import React from 'react';
259299
import PropTypes from 'prop-types';
260300
import {omit} from 'ramda';
301+
${customImport}
261302
262303
/**
263304
* ${Component} is a wrapper for the <${element}> HTML5 element.${customDoc}
264305
* For detailed attribute info see:
265306
* https://developer.mozilla.org/en-US/docs/Web/HTML/Element/${element}
266307
*/
267308
const ${Component} = (props) => {
268-
const dataAttributes = {};
309+
const extraProps = {};
269310
if(props.loading_state && props.loading_state.is_loading) {
270-
dataAttributes['data-dash-is-loading'] = true;
311+
extraProps['data-dash-is-loading'] = true;
271312
}
272-
313+
${customCode}
273314
/* remove unnecessary onClick event listeners */
274315
const isStatic = props.disable_n_clicks || !props.id;
275316
return (
@@ -280,8 +321,8 @@ const ${Component} = (props) => {
280321
n_clicks_timestamp: Date.now()
281322
})
282323
})}
283-
{...omit(['n_clicks', 'n_clicks_timestamp', 'loading_state', 'setProps', 'disable_n_clicks'], props)}
284-
{...dataAttributes}
324+
{...omit(['n_clicks', 'n_clicks_timestamp', 'loading_state', 'setProps', 'disable_n_clicks', 'href'], props)}
325+
{...extraProps}
285326
>
286327
{props.children}
287328
</${element}>

dash/dash-renderer/src/TreeContainer.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
pathOr,
2424
type
2525
} from 'ramda';
26-
import {notifyObservers, updateProps} from './actions';
26+
import {notifyObservers, updateProps, onError} from './actions';
2727
import isSimpleComponent from './isSimpleComponent';
2828
import {recordUiEdit} from './persistence';
2929
import ComponentErrorBoundary from './components/error/ComponentErrorBoundary.react';
@@ -138,9 +138,18 @@ class BaseTreeContainer extends Component {
138138
const {id} = oldProps;
139139
const changedProps = pickBy(
140140
(val, key) => !equals(val, oldProps[key]),
141-
newProps
141+
dissoc('_dash_error', newProps)
142142
);
143143

144+
if (newProps._dash_error) {
145+
_dashprivate_dispatch(
146+
onError({
147+
type: 'frontEnd',
148+
error: newProps._dash_error
149+
})
150+
);
151+
}
152+
144153
if (!isEmpty(changedProps)) {
145154
_dashprivate_dispatch((dispatch, getState) => {
146155
const {graphs} = getState();
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
from dash import Dash, html, dcc
2+
3+
4+
def test_xss001_banned_protocols(dash_duo):
5+
app = Dash()
6+
7+
app.layout = html.Div(
8+
[
9+
dcc.Link("dcc-link", href="javascript:alert(1)", id="dcc-link"),
10+
html.Br(),
11+
html.A(
12+
"html.A", href='javascr\nipt:alert(1);console.log("xss");', id="html-A"
13+
),
14+
html.Br(),
15+
html.Form(
16+
[
17+
html.Button(
18+
"form-action",
19+
formAction="javascript:alert('form-action')",
20+
id="button-form-action",
21+
),
22+
html.Button("submit", role="submit"),
23+
],
24+
action='javascript:alert(1);console.log("xss");',
25+
id="form",
26+
),
27+
html.Iframe(src='javascript:alert("iframe")', id="iframe-src"),
28+
html.ObjectEl(data='javascript:alert("data-object")', id="object-data"),
29+
html.Embed(src='javascript:alert("embed")', id="embed-src"),
30+
# older browser
31+
html.Img(src="javascript:alert('img-sr')", id="img-src"),
32+
]
33+
)
34+
35+
dash_duo.start_server(app)
36+
37+
for element_id, prop in (
38+
("#dcc-link", "href"),
39+
("#html-A", "href"),
40+
("#iframe-src", "src"),
41+
("#object-data", "data"),
42+
("#embed-src", "src"),
43+
("#button-form-action", "formAction"),
44+
("#img-src", "src"),
45+
):
46+
47+
element = dash_duo.find_element(element_id)
48+
assert (
49+
element.get_attribute(prop) == "about:blank"
50+
), f"Failed prop: {element_id}.{prop}"

0 commit comments

Comments
 (0)