Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend safeHref #58

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ setClassNamePrefix('📦')

### Safe `href`s

By default `ui-box` does not ensure that urls use safe protocols when passed to an element. But we built this functionality into `ui-box` to protect the end users of the products you are building. You can alter this by using `configureSafeHref({enabled?: boolean, origin?: string})`. This will ensure that only safe protocols are used (`http:`, `https:`, `mailto:`, `tel:`, and `data:`) and that the correct `rel` values are added (`noopener`, `noreferrer`(for external links)).
By default `ui-box` does not ensure that urls use safe protocols when passed to an element. But we built this functionality into `ui-box` to protect the end users of the products you are building. You can alter this by using `configureSafeHref({enabled?: boolean, origin?: string, additionalProtocols?: string[]})`. This will ensure that only safe protocols are used (`http:`, `https:`, `mailto:`, and `tel:`), that the correct `rel` values are added (`noopener`, `noreferrer`(for external links)), and any additional protocols passed are treated as safe.

```js
import { configureSafeHref } from 'ui-box'
Expand All @@ -320,14 +320,16 @@ configureSafeHref({
import { configureSafeHref } from 'ui-box'
configureSafeHref({
enabled: true
origin: 'https://app.segmentio.com',
origin: 'https://app.segmentio.com',
additionalProtocols: ['data:'],
})
```

Additionally you can overwrite the behavoir on an individual component basis using the prop `allowUnsafeHref`
Additionally you can overwrite the behavoir on an individual component basis using the prop `allowUnsafeHref` and `allowProtocols`. Setting `allowUnsafeHref` completely bypasses all safeHref functionality (protocol checks, rel checks). Setting `allowProtocols` adds the contents of a string array to the allowed protocols.

```jsx
<Box is="a" href="javascript:alert('hi')" allowUnsafeHref={true}>This is unsafe</Box>
<Box is="a" href="data:text/html,<html><h1>Hi</h1><script>alert('hi')</script></html>" allowProtocols={['data:']}>This is unsafe</Box>
```

### Server side rendering
Expand Down
15 changes: 12 additions & 3 deletions src/box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import PropTypes from 'prop-types'
import {BoxComponent} from './types/box-types'
import {propTypes} from './enhancers'
import enhanceProps from './enhance-props'
import {extractAnchorProps, getUseSafeHref} from './utils/safeHref'
import {extractAnchorProps, getUseSafeHref, HrefData} from './utils/safeHref'

const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, ...props }) => {
const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, allowProtocols, ...props }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add these to the PropTypes as well ( allowUnsafeHref and allowProtocols)

// Convert the CSS props to class names (and inject the styles)
const {className, enhancedProps: parsedProps} = enhanceProps(props)

Expand All @@ -22,7 +22,16 @@ const Box: BoxComponent = ({ is = 'div', innerRef, children, allowUnsafeHref, ..
*/
const safeHrefEnabled = (typeof allowUnsafeHref === 'boolean' ? !allowUnsafeHref : getUseSafeHref()) && is === 'a' && parsedProps.href
if (safeHrefEnabled) {
const {safeHref, safeRel} = extractAnchorProps(parsedProps.href, parsedProps.rel)
const hrefData:HrefData = {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: What's the motivation for explicitly marking the type here? What happens if you remove :HrefData?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we opt to keep this as-is, I would suggest a space between hrefData: and HrefData.

href: parsedProps.href,
rel: parsedProps.rel,
}

if (allowProtocols && allowProtocols.length > 0) {
hrefData.allowProtocols = allowProtocols
}

const {safeHref, safeRel} = extractAnchorProps(hrefData)
parsedProps.href = safeHref
parsedProps.rel = safeRel
}
Expand Down
5 changes: 5 additions & 0 deletions src/types/box-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ export type BoxProps<T extends Is> = InheritedProps<T> &
* Allows the high level value of safeHref to be overwritten on an individual component basis
*/
allowUnsafeHref?: boolean

/**
* Allows additional protocols to be considered safe
*/
allowProtocols?: Array<string>
}

export interface BoxComponent {
Expand Down
41 changes: 29 additions & 12 deletions src/utils/safeHref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,22 @@ export interface URLInfo {
sameOrigin: boolean
}

export interface HrefData {
href: string
rel: string
allowProtocols?: string[]
}

export interface SafeHrefConfigObj {
enabled?: boolean
origin?: string
additionalProtocols?: string[]
}

const PROTOCOL_REGEX = /^[a-z]+:/
const ORIGIN_REGEX = /^(?:[a-z]+:?:)?(?:\/\/)?([^\/\?]+)/
const safeProtocols: string[] = ['http:', 'https:', 'mailto:', 'tel:']
let customProtocols:Array<string> = []
let useSafeHref = false
let globalOrigin = typeof window !== 'undefined' ? window.location.origin : false

Expand All @@ -21,40 +30,45 @@ export function configureSafeHref(configObject: SafeHrefConfigObj) {
if (configObject.origin) {
globalOrigin = configObject.origin
}

if (configObject.additionalProtocols && configObject.additionalProtocols.length) {
customProtocols.push(...configObject.additionalProtocols)
}
}

export function getUseSafeHref(): boolean {
return useSafeHref
}

export function getURLInfo(url: string): URLInfo {
/**
* An array of the safely allowed url protocols
*/
const safeProtocols = ['http:', 'https:', 'mailto:', 'tel:', 'data:']
export function resetCustomProtocols() {
customProtocols = []
}

export function getURLInfo(url: string, allowProtocols: Array<string>): URLInfo {
/**
* - Find protocol of URL or set to 'relative'
* - Find origin of URL
* - Determine if sameOrigin
* - Determine if protocol of URL is safe
*/
const protocolResult = url.match(PROTOCOL_REGEX)
const originResult = url.match(ORIGIN_REGEX)
const cleanedUrl = url.trim()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might we worth putting a comment motivating why this is a sufficient "cleanup" step? For instance a link to:

https://html.spec.whatwg.org/multipage/links.html#links-created-by-a-and-area-elements

I can confirm that this is a sufficient cleanup, at least in Node:

> ["\u0009", "\u000A", "\u000C", "\u000D", "\u0020"].map(s => s.trim())
[ '', '', '', '', '' ]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use url.parse (https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost) and then grab the pieces that you need.

const protocolResult = cleanedUrl.match(PROTOCOL_REGEX)
const originResult = cleanedUrl.match(ORIGIN_REGEX)
const urlProtocol = protocolResult ? protocolResult[0] : 'relative'
let sameOrigin = urlProtocol === 'relative'
if (!sameOrigin && globalOrigin) {
sameOrigin = globalOrigin === (originResult && originResult[0])
}

const isSafeProtocol = sameOrigin ? true : safeProtocols.includes(urlProtocol)
const allowedProtocols = [...safeProtocols, ...customProtocols, ...allowProtocols]
const isSafeProtocol = sameOrigin ? true : allowedProtocols.includes(urlProtocol)
if (!isSafeProtocol) {
/**
* If the url is unsafe, put a error in the console, and return the URLInfo object
* with the value of url being `undefined`
*/
console.error(
'📦 `href` passed to anchor tag is unsafe. Because of this, the `href` on the element was not set. Please review the safe href documentation if you have questions.',
'📦 ui-box: `href` passed to anchor tag is unsafe. Because of this, the `href` on the element was not set. Please review the safe href documentation if you have questions.',
'https://www.github.com/segmentio/ui-box'
)
return {
Expand All @@ -67,16 +81,19 @@ export function getURLInfo(url: string): URLInfo {
* If the url is safe, return the url and origin
*/
return {
url,
url: cleanedUrl,
sameOrigin
}
}

export function extractAnchorProps(href: string, rel: string) {
export function extractAnchorProps(hrefData: HrefData) {
const {href, rel} = hrefData
const allowProtocols = hrefData.allowProtocols && hrefData.allowProtocols.length ? hrefData.allowProtocols : []

/**
* Get url info and update href
*/
const urlInfo = getURLInfo(href)
const urlInfo = getURLInfo(href, allowProtocols)
const safeHref = urlInfo.url

/**
Expand Down
58 changes: 58 additions & 0 deletions test/utils/safeHref.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import test from 'ava'
import {extractAnchorProps, configureSafeHref, resetCustomProtocols} from '../../src/utils/safeHref'

test('Allows safe protocols', t => {
configureSafeHref({
enabled: true
})

const {safeHref} = extractAnchorProps({
href: 'https://www.apple.com',
rel: ''
})

t.assert(safeHref === 'https://www.apple.com')
})

test('Rejects unsafe protocols', t => {
const {safeHref} = extractAnchorProps({
href: 'javascript:alert("hi")',
rel: ''
})

t.assert(safeHref === undefined)
})

test('Rejects unsafe protocols with whitespace', t => {
const {safeHref} = extractAnchorProps({
href: ' javascript:alert("hi")',
rel: ''
})

t.assert(safeHref === undefined)
})

test('Allows custom protocol', t => {
configureSafeHref({
additionalProtocols: ['data:']
})

const {safeHref} = extractAnchorProps({
href: 'data:text/html,<html><h1>Hi</h1></html>',
rel: ''
})

resetCustomProtocols()

t.assert(safeHref === 'data:text/html,<html><h1>Hi</h1></html>')
})

test('Allows individual level custom protocol', t => {
const {safeHref} = extractAnchorProps({
href: 'data:text/html,<html><h1>Hi</h1></html>',
rel: '',
allowProtocols: ['data:']
})

t.assert(safeHref === 'data:text/html,<html><h1>Hi</h1></html>')
})
4 changes: 3 additions & 1 deletion tools/story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ storiesOf('Box', module)
})
.add('safe `href`', () => {
configureSafeHref({
enabled: true
enabled: true,
})
return (
<Box paddingTop={30} borderTop="1px solid" marginTop={30}>
Expand All @@ -45,6 +45,8 @@ storiesOf('Box', module)
<Box is="a" href="http://localhost:9009/test">Same Origin Link</Box>
<Box is="a" href="https://apple.com">External Link</Box>
<Box is="a" href="javascript:alert('hi')">Javascript protocol Link</Box>
<Box is="a" href=" data:text/html,<html><h1>Hi</h1></html>">Data protocol Link</Box>
<Box is="a" href=" data:text/html,<html><h1>Hi</h1><script>alert('hi')</script></html>" allowProtocols={['data:']}>Allow protocol Link</Box>
<Box is="a" href="javascript:alert('hi')" allowUnsafeHref={true}>Overwride Safe Href</Box>
</Box>
)
Expand Down