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

Commit

Permalink
Strip cookies from requests fetch favicons
Browse files Browse the repository at this point in the history
Fixes #14250
Auditors: @diracdeltas

Test Plan:
Manually navigate to  https://www.grc.com/cookies/forensics.htm and observe no problems in the report
Navigate to a website with a favicon (such as github.com) and observe the favicon rendering
  • Loading branch information
Slava committed May 30, 2018
1 parent 21f24d0 commit 41e3ffd
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 9 deletions.
24 changes: 22 additions & 2 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const downloadStates = require('../js/constants/downloadStates')
const urlParse = require('./common/urlParse')
const getSetting = require('../js/settings').getSetting
const appUrlUtil = require('../js/lib/appUrlUtil')
const faviconUtil = require('./renderer/lib/faviconUtil')
const siteSettings = require('../js/state/siteSettings')
const settings = require('../js/constants/settings')
const userPrefs = require('../js/state/userPrefs')
Expand Down Expand Up @@ -57,6 +58,11 @@ const registeredSessions = {}
*/
const permissionCallbacks = {}

/**
* A set to keep track of URLs fetching favicons that need special treatment later
*/
const faviconURLs = new Set()

const getBraverySettingsForUrl = (url, appState, isPrivate) => {
const cachedBraverySettings = getBraverySettingsCache(url, isPrivate)
if (cachedBraverySettings) {
Expand Down Expand Up @@ -109,13 +115,21 @@ function registerForBeforeRequest (session, partition) {
}
}

const url = details.url
// filter out special urls for fetching favicons
if (faviconUtil.isWrappedFaviconUrl(url)) {
const redirectURL = faviconUtil.unwrapFaviconUrl(url)
faviconURLs.add(redirectURL)
muonCb({ redirectURL })
return
}

if (shouldIgnoreUrl(details)) {
muonCb({})
return
}

const firstPartyUrl = module.exports.getMainFrameUrl(details)
const url = details.url
// this can happen if the tab is closed and the webContents is no longer available
if (!firstPartyUrl) {
muonCb({ cancel: true })
Expand Down Expand Up @@ -301,9 +315,15 @@ function registerForBeforeSendHeaders (session, partition) {
const isPrivate = module.exports.isPrivate(partition)

session.webRequest.onBeforeSendHeaders(function (details, muonCb) {
// strip cookies from all requests fetching favicons
if (faviconURLs.has(details.url)) {
faviconURLs.delete(details.url)
delete details.requestHeaders['Cookie']
}

// Using an electron binary which isn't from Brave
if (shouldIgnoreUrl(details)) {
muonCb({})
muonCb({ requestHeaders: details.requestHeaders })
return
}

Expand Down
11 changes: 11 additions & 0 deletions app/renderer/lib/faviconUtil.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* 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/. */

const suffix = '#brave-favicon-fragment'

module.exports = {
wrapFaviconUrl: (url) => `${url}${suffix}`,
unwrapFaviconUrl: (url) => url.substring(0, url.length - suffix.length),
isWrappedFaviconUrl: (url) => url.endsWith(suffix)
}
17 changes: 10 additions & 7 deletions app/renderer/rendererTabEvents.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const siteSettingsState = require('../common/state/siteSettingsState')
const contextMenuState = require('../common/state/contextMenuState')

const domUtil = require('./lib/domUtil')
const faviconUtil = require('./lib/faviconUtil')
const imageUtil = require('../../js/lib/imageUtil')
const historyUtil = require('../common/lib/historyUtil')
const UrlUtil = require('../../js/lib/urlutil')
Expand Down Expand Up @@ -111,13 +112,15 @@ const api = module.exports = {
}
case 'page-favicon-updated': {
if (e.favicons &&
e.favicons.length > 0 &&
// Favicon changes lead to recalculation of top site data so only fire
// this when needed. Some sites update favicons very frequently.
e.favicons[0] !== frame.get('icon')) {
imageUtil.getWorkingImageUrl(e.favicons[0], (error) => {
windowActions.setFavicon(frame, error ? null : e.favicons[0])
})
e.favicons.length > 0) {
const url = faviconUtil.wrapFaviconUrl(e.favicons[0])
// Favicon changes lead to recalculation of top site data so only fire
// this when needed. Some sites update favicons very frequently.
if (url !== frame.get('icon')) {
imageUtil.getWorkingImageUrl(url, (error) => {
windowActions.setFavicon(frame, error ? null : url)
})
}
}
break
}
Expand Down

0 comments on commit 41e3ffd

Please sign in to comment.