Sanitize href props with xss vulnerability V2 (#1000)

AnnMarieW · tcbegley · web-flow · commit 86a883b607a9 · 2024-02-11T00:09:49.000Z
* Sanitize href props with xss vulnerability V2

* Sanitize href props with xss vulnerability V2

* Add @testing-library/dom

* add test

* fixed test

* lint

* lint

* fixed warning

* Use pytest mark parameterise

* Refactor prop destructuring

---------

Co-authored-by: tcbegley &lt;tomcbegley@gmail.com&gt;
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -39,6 +39,7 @@
     "@babel/plugin-transform-runtime": "^7.15.0",
     "@babel/preset-env": "^7.15.0",
     "@babel/preset-react": "^7.14.5",
+    "@testing-library/dom": "^9.3.4",
     "@testing-library/jest-dom": "^5.14.1",
     "@testing-library/react": "^12.1.1",
     "@testing-library/user-event": "^13.2.1",
@@ -56,6 +57,7 @@
     "webpack-dev-server": "^4.7.4"
   },
   "dependencies": {
+    "@braintree/sanitize-url": "^7.0.0",
     "@plotly/dash-component-plugins": "^1.2.0",
     "classnames": "^2.2.6",
     "fast-isnumeric": "^1.1.3",
diff --git a/src/components/badge/Badge.js b/src/components/badge/Badge.js
@@ -4,6 +4,7 @@ import {omit} from 'ramda';
 import RBBadge from 'react-bootstrap/Badge';
 import Link from '../../private/Link';
 import {bootstrapColors} from '../../private/BootstrapColors';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Badges can be used to add counts or labels to other components.
@@ -22,6 +23,8 @@ const Badge = props => {
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const incrementClicks = () => {
     if (setProps) {
       setProps({
@@ -36,8 +39,8 @@ const Badge = props => {
 
   return (
     <RBBadge
-      as={href && Link}
-      href={href}
+      as={sanitizedUrl && Link}
+      href={sanitizedUrl}
       bg={isBootstrapColor ? color : null}
       text={text_color}
       className={class_name || className}
diff --git a/src/components/breadcrumb/Breadcrumb.js b/src/components/breadcrumb/Breadcrumb.js
@@ -3,10 +3,33 @@ import PropTypes from 'prop-types';
 import RBBreadcrumb from 'react-bootstrap/Breadcrumb';
 
 import Link from '../../private/Link';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Use breadcrumbs to create a navigation breadcrumb in your app.
  */
+
+const BreadcrumbItem = ({
+  href,
+  setProps,
+  external_link,
+  label,
+  ...otherProps
+}) => {
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
+  return (
+    <RBBreadcrumb.Item
+      linkAs={sanitizedUrl && Link}
+      href={sanitizedUrl}
+      linkProps={sanitizedUrl && {external_link}}
+      {...otherProps}
+    >
+      {label}
+    </RBBreadcrumb.Item>
+  );
+};
+
 const Breadcrumb = ({
   items,
   tag,
@@ -16,6 +39,7 @@ const Breadcrumb = ({
   item_class_name,
   itemClassName,
   item_style,
+  setProps,
   ...otherProps
 }) => (
   <RBBreadcrumb
@@ -27,16 +51,12 @@ const Breadcrumb = ({
     {...otherProps}
   >
     {(items || []).map((item, idx) => (
-      <RBBreadcrumb.Item
+      <BreadcrumbItem
         key={`${item.value}${idx}`}
-        active={item.active}
-        linkAs={item.href && Link}
         className={item_class_name || itemClassName}
-        href={item.href}
-        linkProps={item.href && {external_link: item.external_link}}
-      >
-        {item.label}
-      </RBBreadcrumb.Item>
+        setProps={setProps}
+        {...item}
+      />
     ))}
   </RBBreadcrumb>
 );
diff --git a/src/components/button/Button.js b/src/components/button/Button.js
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import {omit} from 'ramda';
 import RBButton from 'react-bootstrap/Button';
 import Link from '../../private/Link';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * A component for creating Bootstrap buttons with different style options. The
@@ -34,6 +35,8 @@ const Button = props => {
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const incrementClicks = () => {
     if (!disabled && setProps) {
       setProps({
@@ -42,7 +45,7 @@ const Button = props => {
       });
     }
   };
-  const useLink = href && !disabled;
+  const useLink = sanitizedUrl && !disabled;
   otherProps[useLink ? 'preOnClick' : 'onClick'] = onClick || incrementClicks;
 
   if (useLink) {
@@ -56,7 +59,7 @@ const Button = props => {
       as={useLink ? Link : 'button'}
       variant={outline ? `outline-${color}` : color}
       type={useLink ? undefined : type}
-      href={disabled ? undefined : href}
+      href={disabled ? undefined : sanitizedUrl}
       disabled={disabled}
       download={useLink ? download : undefined}
       name={useLink ? undefined : name}
diff --git a/src/components/card/CardLink.js b/src/components/card/CardLink.js
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import {omit} from 'ramda';
 import RBCard from 'react-bootstrap/Card';
 import Link from '../../private/Link';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Use card link to add consistently styled links to your cards. Links can be
@@ -15,12 +16,16 @@ const CardLink = props => {
     disabled,
     className,
     class_name,
+    href,
+    setProps,
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const incrementClicks = () => {
-    if (!disabled && props.setProps) {
-      props.setProps({
+    if (!disabled && setProps) {
+      setProps({
         n_clicks: props.n_clicks + 1,
         n_clicks_timestamp: Date.now()
       });
@@ -35,8 +40,9 @@ const CardLink = props => {
       as={Link}
       preOnClick={incrementClicks}
       disabled={disabled}
+      href={sanitizedUrl}
       className={class_name || className}
-      {...omit(['setProps', 'n_clicks', 'n_clicks_timestamp'], otherProps)}
+      {...omit(['n_clicks', 'n_clicks_timestamp'], otherProps)}
     >
       {children}
     </RBCard.Link>
diff --git a/src/components/dropdownmenu/DropdownMenuItem.js b/src/components/dropdownmenu/DropdownMenuItem.js
@@ -5,6 +5,7 @@ import RBDropdown from 'react-bootstrap/Dropdown';
 
 import Link from '../../private/Link';
 import {DropdownMenuContext} from '../../private/DropdownMenuContext';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Use DropdownMenuItem to build up the content of a DropdownMenu.
@@ -26,6 +27,8 @@ const DropdownMenuItem = props => {
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const context = useContext(DropdownMenuContext);
 
   const handleClick = e => {
@@ -40,7 +43,7 @@ const DropdownMenuItem = props => {
     }
   };
 
-  const useLink = href && !disabled;
+  const useLink = sanitizedUrl && !disabled;
   otherProps[useLink ? 'preOnClick' : 'onClick'] = e => handleClick(e);
 
   if (header) {
@@ -52,7 +55,7 @@ const DropdownMenuItem = props => {
   return (
     <RBDropdown.Item
       as={useLink ? Link : 'button'}
-      href={useLink ? href : undefined}
+      href={useLink ? sanitizedUrl : undefined}
       disabled={disabled}
       target={useLink ? target : undefined}
       className={class_name || className}
diff --git a/src/components/listgroup/ListGroupItem.js b/src/components/listgroup/ListGroupItem.js
@@ -4,6 +4,7 @@ import {omit} from 'ramda';
 import RBListGroupItem from 'react-bootstrap/ListGroupItem';
 import Link from '../../private/Link';
 import {bootstrapColors} from '../../private/BootstrapColors';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Create a single item in a `ListGroup`.
@@ -24,6 +25,8 @@ const ListGroupItem = props => {
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const incrementClicks = () => {
     if (!disabled && setProps) {
       setProps({
@@ -33,13 +36,13 @@ const ListGroupItem = props => {
     }
   };
   const isBootstrapColor = bootstrapColors.has(color);
-  const useLink = href && !disabled;
+  const useLink = sanitizedUrl && !disabled;
   otherProps[useLink ? 'preOnClick' : 'onClick'] = incrementClicks;
 
   return (
     <RBListGroupItem
       as={useLink ? Link : 'li'}
-      href={href}
+      href={sanitizedUrl}
       target={useLink ? target : undefined}
       disabled={disabled}
       variant={isBootstrapColor ? color : null}
diff --git a/src/components/nav/NavLink.js b/src/components/nav/NavLink.js
@@ -4,6 +4,7 @@ import {omit} from 'ramda';
 import classNames from 'classnames';
 import {History} from '@plotly/dash-component-plugins';
 import Link from '../../private/Link';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Add a link to a `Nav`. Can be used as a child of `NavItem` or of `Nav`
@@ -24,11 +25,13 @@ const NavLink = props => {
     ...otherProps
   } = props;
 
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
+
   const pathnameToActive = pathname => {
     setLinkActive(
       active === true ||
-        (active === 'exact' && pathname === href) ||
-        (active === 'partial' && pathname.startsWith(href))
+        (active === 'exact' && pathname === sanitizedUrl) ||
+        (active === 'partial' && pathname.startsWith(sanitizedUrl))
     );
   };
 
@@ -61,7 +64,7 @@ const NavLink = props => {
       className={classes}
       disabled={disabled}
       preOnClick={incrementClicks}
-      href={href}
+      href={sanitizedUrl}
       {...omit(['n_clicks_timestamp'], otherProps)}
       data-dash-is-loading={
         (loading_state && loading_state.is_loading) || undefined
diff --git a/src/components/nav/NavbarBrand.js b/src/components/nav/NavbarBrand.js
@@ -3,17 +3,28 @@ import PropTypes from 'prop-types';
 import {omit} from 'ramda';
 import RBNavbarBrand from 'react-bootstrap/NavbarBrand';
 import Link from '../../private/Link';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 /**
  * Call out attention to a brand name or site title within a navbar.
  */
 const NavbarBrand = props => {
-  const {children, loading_state, className, class_name, ...otherProps} = props;
+  const {
+    children,
+    loading_state,
+    className,
+    class_name,
+    href,
+    setProps,
+    ...otherProps
+  } = props;
+  const sanitizedUrl = sanitizeAndCheckUrl(href, setProps);
   return (
     <RBNavbarBrand
       className={class_name || className}
-      {...omit(['setProps'], otherProps)}
-      as={props.href ? Link : 'span'}
+      {...otherProps}
+      as={sanitizedUrl ? Link : 'span'}
+      href={sanitizedUrl}
       data-dash-is-loading={
         (loading_state && loading_state.is_loading) || undefined
       }
diff --git a/src/components/nav/NavbarSimple.js b/src/components/nav/NavbarSimple.js
@@ -4,6 +4,7 @@ import {omit} from 'ramda';
 import RBNavbar from 'react-bootstrap/Navbar';
 import RBContainer from 'react-bootstrap/Container';
 import {bootstrapColors} from '../../private/BootstrapColors';
+import {sanitizeAndCheckUrl} from '../../private/util';
 
 import Nav from './Nav';
 import NavbarBrand from './NavbarBrand';
@@ -29,8 +30,12 @@ const NavbarSimple = props => {
     loading_state,
     className,
     class_name,
+    setProps,
     ...otherProps
   } = props;
+
+  const sanitizedUrl = sanitizeAndCheckUrl(brand_href, setProps);
+
   const isBootstrapColor = bootstrapColors.has(color);
 
   const [navbarOpen, setNavbarOpen] = useState(false);
@@ -52,7 +57,7 @@ const NavbarSimple = props => {
       <RBContainer fluid={fluid}>
         {brand && (
           <NavbarBrand
-            href={brand_href}
+            href={sanitizedUrl}
             style={brand_style}
             external_link={brand_external_link}
           >
diff --git a/src/private/util.js b/src/private/util.js
@@ -1,4 +1,6 @@
 import {type} from 'ramda';
+import React, {useEffect, useMemo} from 'react';
+import {sanitizeUrl} from '@braintree/sanitize-url';
 
 const parseChildrenToArray = children => {
   if (children && !Array.isArray(children)) {
@@ -73,4 +75,26 @@ const stringifyId = id => {
   return '{' + parts.join(',') + '}';
 };
 
-export {parseChildrenToArray, resolveChildProps, sanitizeOptions, stringifyId};
+const sanitizeAndCheckUrl = (href, setProps) => {
+  const sanitizedUrl = useMemo(() => {
+    return href ? sanitizeUrl(href) : undefined;
+  }, [href]);
+
+  useEffect(() => {
+    if (sanitizedUrl && sanitizedUrl !== href) {
+      setProps({
+        _dash_error: new Error(`Dangerous link detected:: ${href}`)
+      });
+    }
+  }, [href, sanitizedUrl]);
+
+  return sanitizedUrl;
+};
+
+export {
+  parseChildrenToArray,
+  resolveChildProps,
+  sanitizeOptions,
+  stringifyId,
+  sanitizeAndCheckUrl
+};
diff --git a/tests/test_positional_args.py b/tests/test_positional_args.py
diff --git a/tests/test_xss_links.py b/tests/test_xss_links.py
