Skip to content

Commit

Permalink
Merge pull request #5286 from brave/fix-9317
Browse files Browse the repository at this point in the history
Uses noopener for Binance URLs, client url validation
  • Loading branch information
ryanml authored Apr 20, 2020
2 parents e3d0202 + a8bd192 commit 8a0594e
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 7 deletions.
37 changes: 37 additions & 0 deletions components/brave_new_tab_ui/binance-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,40 @@ export const getUSDPrice = (accountBTCBalance: string, btcUSDPrice: string) => {

return (btcUSDPriceNumber * btcBalanceNumber).toFixed(2)
}

export const isValidClientURL = (url: string) => {
if (!url) {
return false
}

let urlObj
try {
urlObj = new URL(url)
} catch (err) {
return false
}

if (urlObj.protocol !== 'https:') {
return false
}

if (urlObj.host !== 'accounts.binance.com') {
return false
}

const { pathname } = urlObj
const pathSplit = pathname.split('/')

// The first level of path is the locale, so the second and
// third levels should be checked for equality to 'oauth' and
// 'authorize' respectively. ex: '/en/oauth/authorize'
if (pathSplit.length !== 4) {
return false
}

if (pathSplit[2] !== 'oauth' || pathSplit[3] !== 'authorize') {
return false
}

return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ class Binance extends React.PureComponent<Props, State> {

connectBinance = () => {
const { binanceClientUrl } = this.props
window.open(binanceClientUrl, '_self')
window.open(binanceClientUrl, '_self', 'noopener')
this.props.onConnectBinance()
}

Expand Down
8 changes: 4 additions & 4 deletions components/brave_new_tab_ui/containers/newTab/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ class NewTabPage extends React.Component<Props, State> {
const refParams = `ref=${refCode}&utm_source=brave`

if (userTLD === 'us') {
window.open(`https://www.binance.us/en/buy-sell-crypto?crypto=${coin}&amount=${amount}&${refParams}`, '_blank')
window.open(`https://www.binance.us/en/buy-sell-crypto?crypto=${coin}&amount=${amount}&${refParams}`, '_blank', 'noopener')
} else {
window.open(`https://www.binance.com/en/buy-sell-crypto?fiat=${fiat}&crypto=${coin}&amount=${amount}&${refParams}`, '_blank')
window.open(`https://www.binance.com/en/buy-sell-crypto?fiat=${fiat}&crypto=${coin}&amount=${amount}&${refParams}`, '_blank', 'noopener')
}
}

Expand Down Expand Up @@ -356,11 +356,11 @@ class NewTabPage extends React.Component<Props, State> {
}

learnMoreRewards = () => {
window.open('https://brave.com/brave-rewards/', '_blank')
window.open('https://brave.com/brave-rewards/', '_blank', 'noopener')
}

learnMoreBinance = () => [
window.open('https://brave.com/binance/', '_blank')
window.open('https://brave.com/binance/', '_blank', 'noopener')
]

setAssetDepositQRCodeSrc = (asset: string, src: string) => {
Expand Down
10 changes: 8 additions & 2 deletions components/brave_new_tab_ui/reducers/new_tab_reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { registerViewCount } from '../api/brandedWallpaper'
import * as preferencesAPI from '../api/preferences'
import * as storage from '../storage/new_tab_storage'
import { getTotalContributions } from '../rewards-utils'
import { getUSDPrice } from '../binance-utils'
import { getUSDPrice, isValidClientURL } from '../binance-utils'

const initialState = storage.load()

Expand Down Expand Up @@ -456,8 +456,14 @@ export const newTabReducer: Reducer<NewTab.State | undefined> = (state: NewTab.S
break

case types.ON_BINANCE_CLIENT_URL:
const { clientUrl } = payload

if (!isValidClientURL(clientUrl)) {
break
}

state = { ...state }
state.binanceState.binanceClientUrl = payload.clientUrl
state.binanceState.binanceClientUrl = clientUrl
break

case types.ON_BTC_USD_PRICE:
Expand Down
36 changes: 36 additions & 0 deletions components/test/brave_new_tab_ui/helpers/binance-utils-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) 2020 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

import * as binanceUtils from '../../../brave_new_tab_ui/binance-utils'

describe('binance utilities tests', () => {
describe('isValidClientURL', () => {
it('handles falsey urls', () => {
expect(binanceUtils.isValidClientURL('')).toBe(false)
expect(binanceUtils.isValidClientURL(null)).toBe(false)
})

it('handles non-url strings', () => {
expect(binanceUtils.isValidClientURL('httpsurl')).toBe(false)
expect(binanceUtils.isValidClientURL('abcdefgh')).toBe(false)
})

it('handles non https protocols', () => {
expect(binanceUtils.isValidClientURL('http://accounts.binance.com/en/oauth/authorize')).toBe(false)
})

it('handles unexpected hosts', () => {
expect(binanceUtils.isValidClientURL('https://accounts.fake-binance.com/en/oauth/authorize')).toBe(false)
})

it('handles unexpected paths', () => {
expect(binanceUtils.isValidClientURL('https://accounts.binance.com/en/oauth/not-authorize')).toBe(false)
})

it('accepts an expected url', () => {
expect(binanceUtils.isValidClientURL('https://accounts.binance.com/en/oauth/authorize?response_type=code&client_id=some_id&redirect_uri=com.brave.binance%3A%2F%2Fauthorization&scope=user%3Aemail%2Cuser%3Aaddress%2Casset%3Abalance%2Casset%3Aocbs&code_challenge=code&code_challenge_method=S256')).toBe(true)
})
})
})

0 comments on commit 8a0594e

Please sign in to comment.