Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Fix websockets bypass in Brave filtering
Browse files Browse the repository at this point in the history
Fix #6997.
Note: this works around the issue of websocket requests being associated with tabId -1, which seems wrong and is why websocket URLs do not show up in the Bravery Panel stats.

Auditors: @bridiver

Test Plan: covered by automated test
  • Loading branch information
diracdeltas committed Feb 8, 2017
1 parent f7a5e17 commit 145d84b
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 2 deletions.
12 changes: 10 additions & 2 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,16 @@ function initForPartition (partition) {
fns.forEach((fn) => { fn(ses, partition, module.exports.isPrivate(partition)) })
}

const filterableProtocols = ['http:', 'https:']
const filterableProtocols = ['http:', 'https:', 'ws:', 'wss:']

function shouldIgnoreUrl (details) {
if (typeof details.url !== 'string') {
return true
}

// internal requests
if (details.tabId === -1) {
if (details.tabId === -1 &&
(details.url.startsWith('http://') || details.url.startsWith('https://'))) {
return true
}

Expand Down Expand Up @@ -738,6 +743,9 @@ module.exports.getMainFrameUrl = (details) => {
if (details.resourceType === 'mainFrame') {
return details.url
}
if (details.tabId === -1 && details.firstPartyUrl) {
return details.firstPartyUrl
}
const tab = webContents.fromTabID(details.tabId)
if (tab && !tab.isDestroyed()) {
return tab.getURL()
Expand Down
1 change: 1 addition & 0 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ class Main extends ImmutableComponent {
windowActions.setActiveFrame(self.props.windowState.getIn(['frames', self.props.windowState.get('frames').size - 1])))

ipc.on(messages.BLOCKED_RESOURCE, (e, blockType, details) => {
// XXX: ws and wss URLs have tabId -1 so frameProps is undefined
const frameProps = frameStateUtil.getFrameByTabId(self.props.windowState, details.tabId)
frameProps && windowActions.setBlockedBy(frameProps, blockType, details.url)
})
Expand Down
51 changes: 51 additions & 0 deletions test/components/braveryPanelTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const Brave = require('../lib/brave')
const messages = require('../../js/constants/messages')
const {urlInput, braveMenu, braveMenuDisabled, adsBlockedStat, adsBlockedControl, showAdsOption, blockAdsOption, braveryPanel, httpsEverywhereStat, noScriptStat, noScriptSwitch, fpSwitch, fpStat, noScriptNavButton} = require('../lib/selectors')
const {getTargetAboutUrl} = require('../../js/lib/appUrlUtil')
const assert = require('assert')

describe('Bravery Panel', function () {
function * setup (client) {
Expand Down Expand Up @@ -268,6 +269,56 @@ describe('Bravery Panel', function () {
.click(blockAdsOption)
.waitForTextValue(adsBlockedStat, '2')
})
it('blocks websocket tracking', function * () {
const url = Brave.server.url('websockets.html')
let time1 = 0
let time2 = 0
yield this.app.client
.waitForDataFile('adblock')
.tabByIndex(0)
.loadUrl(url)
.waitUntil(function () {
return this.getText('#result')
.then((result) => {
return result === 'success'
})
})
.waitUntil(function () {
// The connection to the websocket URL is blocked by adblock, so
// onerror fires quickly
return this.getText('#errorTime')
.then((time) => {
time = Number(time)
if (time > 0) {
time1 = time
return true
}
return false
})
})
// TODO: Show websocket URLs in Bravery Stats
.openBraveMenu(braveMenu, braveryPanel)
.click(adsBlockedControl)
.waitForVisible(showAdsOption)
.click(showAdsOption)
.tabByIndex(0)
.waitUntil(function () {
// The connection to the websocket URL is allowed by adblock, so
// onerror fires only after the connection is attempted
return this.getText('#errorTime')
.then((time) => {
time = Number(time)
if (time > 0) {
time2 = time
return true
}
return false
})
})
// Assume blocking websockets is working if the time difference between
// onerror firing with adblock on vs off is sufficiently high.
assert(time2 - time1 > 100)
})
// TODO(bridiver) using slashdot won't provide reliable results so we should
// create our own iframe page with urls we expect to be blocked
it('detects blocked elements in iframe in private tab', function * () {
Expand Down
14 changes: 14 additions & 0 deletions test/fixtures/websockets.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<div id='result'></div>
<div id='errorTime'></div>
<script>
// This should get blocked by adblock if it is on
var exampleSocket = new WebSocket("ws://ag.innovid.com/dv/sync?tid=2");
exampleSocket.onerror = (e) => {
document.getElementById('errorTime').innerText = e.timeStamp
}
// This should be a valid connection
var ws = new WebSocket("wss://wss.websocketstest.com:443/service")
ws.onopen = function() {
document.getElementById('result').innerText = 'success'
}
</script>

0 comments on commit 145d84b

Please sign in to comment.