From 8d8b4f132326696ba490dbc12f0d6e25dd81647a Mon Sep 17 00:00:00 2001 From: yan Date: Thu, 10 Sep 2020 11:27:32 -0700 Subject: [PATCH] Remove non-locale uses of innerHTML and add npm script to check for Trusted Type violations. locale strings will be fixed in a separate PR. fix https://github.com/brave/brave-browser/issues/11686 --- .../settings/brave_overrides/about_page.js | 9 +- .../settings/brave_overrides/basic_page.js | 113 ++++++++++++------ .../brave_overrides/import_data_dialog.js | 25 ++-- .../settings/brave_overrides/people_page.js | 2 +- .../settings/brave_overrides/settings_menu.js | 54 ++++++--- build/commands/scripts/checkSecurity.js | 29 +++++ .../extension/brave_extension/webstore.ts | 4 +- .../brave_rewards/content_scripts/github.ts | 10 +- .../brave_rewards/content_scripts/reddit.ts | 9 +- .../brave_rewards/content_scripts/twitter.ts | 3 +- .../resources/brave_flags_overrides.js | 26 ++-- package.json | 3 +- ui/webui/resources/polymer_overriding.js | 9 +- 13 files changed, 204 insertions(+), 92 deletions(-) create mode 100644 build/commands/scripts/checkSecurity.js diff --git a/browser/resources/settings/brave_overrides/about_page.js b/browser/resources/settings/brave_overrides/about_page.js index 2aa5e3627063..aafe6c1c347a 100644 --- a/browser/resources/settings/brave_overrides/about_page.js +++ b/browser/resources/settings/brave_overrides/about_page.js @@ -14,7 +14,14 @@ RegisterPolymerTemplateModifications({ if (!version) { console.error('[Brave Settings Overrides] Could not find version div') } - version.innerHTML = '' + version.innerHTML + '' + const parent = version.parentNode + const wrapper = document.createElement('a') + wrapper.setAttribute('id', 'release-notes') + wrapper.setAttribute('target', '_blank') + wrapper.setAttribute('rel', 'noopener noreferrer') + wrapper.setAttribute('href', 'https://brave.com/latest/') + parent.replaceChild(wrapper, version) + wrapper.appendChild(version) } } }) diff --git a/browser/resources/settings/brave_overrides/basic_page.js b/browser/resources/settings/brave_overrides/basic_page.js index d7a3884175bc..3671ad287ac8 100644 --- a/browser/resources/settings/brave_overrides/basic_page.js +++ b/browser/resources/settings/brave_overrides/basic_page.js @@ -28,6 +28,26 @@ export function getSectionElement (templateContent, sectionName) { return sectionEl } +/** + * Creates a settings-section element with a single child and returns it. + * @param {string} sectionName - value of the section attribute + * @param {string} titleName - loadTimeData key for page-title + * @param {string} childName - name of child element + * @param {Object} childAttributes - key-value pairs of child element attributes + * @returns {Element} + */ +function createSectionElement (sectionName, titleName, childName, childAttributes) { + const el = document.createElement('settings-section') + el.setAttribute('page-title', loadTimeData.getString(titleName)) + el.setAttribute('section', sectionName) + const child = document.createElement(childName) + for (const attribute in childAttributes) { + child.setAttribute(attribute, childAttributes[attribute]) + } + el.appendChild(child) + return el +} + RegisterStyleOverride( 'settings-basic-page', html` @@ -94,67 +114,86 @@ RegisterPolymerTemplateModifications({ sectionGetStarted.setAttribute('is', 'dom-if') sectionGetStarted.setAttribute('restamp', true) sectionGetStarted.setAttribute('if', '[[showPage_(pageVisibility.getStarted)]]') - sectionGetStarted.innerHTML = ` - - - - ` + sectionGetStarted.content.appendChild(createSectionElement( + 'getStarted', + 'braveGetStartedTitle', + 'brave-settings-getting-started', + { + prefs: '{{prefs}}', + 'page-visibility': '[[pageVisibility]]' + } + )) const sectionExtensions = document.createElement('template') sectionExtensions.setAttribute('is', 'dom-if') sectionExtensions.setAttribute('restamp', true) sectionExtensions.setAttribute('if', '[[showPage_(pageVisibility.extensions)]]') - sectionExtensions.innerHTML = ` - - - - ` + sectionExtensions.content.appendChild(createSectionElement( + 'extensions', + 'braveDefaultExtensions', + 'settings-brave-default-extensions-page', + { + prefs: '{{prefs}}' + } + )) const sectionSync = document.createElement('template') sectionSync.setAttribute('is', 'dom-if') sectionSync.setAttribute('restamp', true) sectionSync.setAttribute('if', '[[showPage_(pageVisibility.braveSync)]]') - sectionSync.innerHTML = ` - - - - ` + sectionSync.content.appendChild(createSectionElement( + 'braveSync', + 'braveSync', + 'settings-brave-sync-page', + {} + )) + const sectionShields = document.createElement('template') sectionShields.setAttribute('is', 'dom-if') sectionShields.setAttribute('restamp', true) sectionShields.setAttribute('if', '[[showPage_(pageVisibility.shields)]]') - sectionShields.innerHTML = ` - - - - ` + sectionShields.content.appendChild(createSectionElement( + 'shields', + 'braveShieldsTitle', + 'settings-default-brave-shields-page', + { + prefs: '{{prefs}}' + } + )) const sectionSocialBlocking = document.createElement('template') sectionSocialBlocking.setAttribute('is', 'dom-if') sectionSocialBlocking.setAttribute('restamp', true) sectionSocialBlocking.setAttribute('if', '[[showPage_(pageVisibility.socialBlocking)]]') - sectionSocialBlocking.innerHTML = ` - - - - ` + sectionSocialBlocking.content.appendChild(createSectionElement( + 'socialBlocking', + 'socialBlocking', + 'settings-social-blocking-page', + { + prefs: '{{prefs}}' + } + )) const sectionHelpTips = document.createElement('template') sectionHelpTips.setAttribute('is', 'dom-if') sectionHelpTips.setAttribute('restamp', true) sectionHelpTips.setAttribute('if', '[[showPage_(pageVisibility.braveHelpTips)]]') - sectionHelpTips.innerHTML = ` - - - - ` + sectionHelpTips.content.appendChild(createSectionElement( + 'braveHelpTips', + 'braveHelpTips', + 'settings-brave-help-tips-page', + { + prefs: '{{prefs}}' + } + )) const sectionNewTab = document.createElement('template') sectionNewTab.setAttribute('is', 'dom-if') sectionNewTab.setAttribute('restamp', true) sectionNewTab.setAttribute('if', '[[showPage_(pageVisibility.newTab)]]') - sectionNewTab.innerHTML = ` - - - - ` + sectionNewTab.content.appendChild(createSectionElement( + 'newTab', + 'braveNewTab', + 'settings-brave-new-tab-page', + { + prefs: '{{prefs}}' + } + )) // Get Started at top basicPageEl.insertAdjacentElement('afterbegin', sectionGetStarted) // Move Appearance item diff --git a/browser/resources/settings/brave_overrides/import_data_dialog.js b/browser/resources/settings/brave_overrides/import_data_dialog.js index 2805810443f2..11c238c3c710 100644 --- a/browser/resources/settings/brave_overrides/import_data_dialog.js +++ b/browser/resources/settings/brave_overrides/import_data_dialog.js @@ -8,21 +8,16 @@ import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js' RegisterPolymerTemplateModifications({ 'settings-import-data-dialog': (templateContent) => { - let checkBoxesParent = templateContent.querySelector('#browserSelect').parentElement - let innerHTML = checkBoxesParent.innerHTML - innerHTML += ` - - - ` - checkBoxesParent.innerHTML = innerHTML + let checkBoxesParent = templateContent.querySelector('#browserSelect').parentElement; + ['extensions', 'payments'].forEach((item) => { + const checkbox = document.createElement('settings-checkbox') + checkbox.setAttribute('hidden', `[[!selected_.${item}]]`) + checkbox.setAttribute('pref', `{{prefs.import_dialog_${item}}}`) + checkbox.setAttribute('label', + I18nBehavior.i18n(`import${item[0].toUpperCase()}${item.slice(1)}`)) + checkbox.setAttribute('no-set-pref', '') + checkBoxesParent.appendChild(checkbox) + }) } }) diff --git a/browser/resources/settings/brave_overrides/people_page.js b/browser/resources/settings/brave_overrides/people_page.js index 3dedd2376219..0f5d0f307711 100644 --- a/browser/resources/settings/brave_overrides/people_page.js +++ b/browser/resources/settings/brave_overrides/people_page.js @@ -55,7 +55,7 @@ RegisterPolymerTemplateModifications({ // remove the google account button const manageGoogleAccount = signinTemplate.content.querySelector('#manage-google-account') if (!manageGoogleAccount) { - console.error('[Brave Settings Overrides] Could not find the google account settings item', templateContent, templateContent.innerHTML) + console.error('[Brave Settings Overrides] Could not find the google account settings item', templateContent, templateContent.textContent) return } manageGoogleAccount.remove() diff --git a/browser/resources/settings/brave_overrides/settings_menu.js b/browser/resources/settings/brave_overrides/settings_menu.js index 91c38246f6b2..6ebad54b4189 100644 --- a/browser/resources/settings/brave_overrides/settings_menu.js +++ b/browser/resources/settings/brave_overrides/settings_menu.js @@ -14,10 +14,11 @@ function createMenuElement (title, href, iconName, pageVisibilitySection) { menuEl.setAttribute('hidden', `[[!pageVisibility.${pageVisibilitySection}]]`) } menuEl.href = href - menuEl.innerHTML = ` - - ${title} - ` + const child = document.createElement('iron-icon') + child.setAttribute('icon', iconName) + menuEl.appendChild(child) + const text = document.createTextNode(title) + menuEl.appendChild(text) return menuEl } @@ -140,7 +141,7 @@ RegisterPolymerTemplateModifications({ // Add title const titleEl = document.createElement('h1') titleEl.id = 'settingsHeader' - titleEl.innerHTML = loadTimeData.getString('settings') + titleEl.textContent = loadTimeData.getString('settings') const topMenuEl = templateContent.querySelector('#topMenu') if (!topMenuEl) { console.error('[Brave Settings Overrides] Could not find topMenu element to add title after') @@ -152,7 +153,7 @@ RegisterPolymerTemplateModifications({ if (!advancedToggle) { console.error('[Brave Settings Overrides] Could not find advancedButton to modify text') } - advancedToggle.innerText = loadTimeData.getString('braveAdditionalSettingsTitle') + advancedToggle.textContent = loadTimeData.getString('braveAdditionalSettingsTitle') // Add 'Get Started' item const peopleEl = getMenuElement(templateContent, '/people') const getStartedEl = createMenuElement( @@ -238,16 +239,37 @@ RegisterPolymerTemplateModifications({ const aboutEl = templateContent.querySelector('#about-menu') if (!aboutEl) { console.error('[Brave Settings Overrides] Could not find about-menu element') + return } - const aboutTitleContent = aboutEl.innerHTML - aboutEl.innerHTML = ` -
- -
-
- ${aboutTitleContent} - v ${loadTimeData.getString('braveProductVersion')} -
- ` + const parent = aboutEl.parentNode + parent.removeChild(aboutEl) + + const newAboutEl = document.createElement('a') + newAboutEl.setAttribute('href', '/help') + newAboutEl.setAttribute('id', aboutEl.id) + + const graphicsEl = document.createElement('div') + graphicsEl.setAttribute('class', 'brave-about-graphic') + + const icon = document.createElement('iron-icon') + icon.setAttribute('icon', 'brave_settings:full-color-brave-lion') + + const metaEl = document.createElement('div') + metaEl.setAttribute('class', 'brave-about-meta') + + const menuLink = document.createElement('span') + menuLink.setAttribute('class', 'brave-about-item brave-about-menu-link-text') + menuLink.textContent = aboutEl.textContent + + const versionEl = document.createElement('span') + versionEl.setAttribute('class', 'brave-about-item brave-about-menu-version') + versionEl.textContent = `v ${loadTimeData.getString('braveProductVersion')}` + + parent.appendChild(newAboutEl) + newAboutEl.appendChild(graphicsEl) + graphicsEl.appendChild(icon) + newAboutEl.appendChild(metaEl) + metaEl.appendChild(menuLink) + metaEl.appendChild(versionEl) } }) diff --git a/build/commands/scripts/checkSecurity.js b/build/commands/scripts/checkSecurity.js new file mode 100644 index 000000000000..708324fa8d6f --- /dev/null +++ b/build/commands/scripts/checkSecurity.js @@ -0,0 +1,29 @@ +// 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/. + +const execSync = require('child_process').execSync + +// Grep for any usage of innerHTML. TODO: add dangerouslySetInnerHTML. +// Ping @security-team before changing this. +const cmd = "git grep --extended-regexp '(innerHTML|document.write)' ':(exclude)*test.cc' ':(exclude)test/*' ':(exclude)*.json' ':(exclude)build/*' ':(exclude)*browsertest*.cc'" + +try { + const stdout = execSync(cmd) + if (stdout.length) { + console.log(stdout.toString()) + // grep returned results, so the test fails. + console.log('checkSecurity failed! Found uses of innerHTML/document.write.') + process.exit(1) + } +} catch (e) { + if (!e.stderr.length && !e.stdout.length && e.status === 1) { + // no grep results, so test passes + console.log('checkSecurity passed.') + process.exit(0) + } else { + console.log('checkSecurity failed. See above for error.') + process.exit(e.status) + } +} diff --git a/components/brave_extension/extension/brave_extension/webstore.ts b/components/brave_extension/extension/brave_extension/webstore.ts index b52c38a32f88..ebc06f1199aa 100644 --- a/components/brave_extension/extension/brave_extension/webstore.ts +++ b/components/brave_extension/extension/brave_extension/webstore.ts @@ -5,9 +5,9 @@ const callback = (mutationsList: MutationRecord[], observer: MutationObserver) = const buttons: NodeListOf = document.querySelectorAll('div.webstore-test-button-label') buttons.forEach((button: Element) => { - const text: string = button.innerHTML + const text: string = button.textContent || '' if (textToMatch.includes(text)) { - button.innerHTML = text.replace('Chrome', 'Brave') + button.textContent = text.replace('Chrome', 'Brave') } }) } diff --git a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/github.ts b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/github.ts index c2ea0a0b8c95..254040a72a72 100644 --- a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/github.ts +++ b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/github.ts @@ -96,7 +96,9 @@ const createBraveTipAction = (elem: Element, getMetaData: (elem: Element) => Rew // Create style element for hover color const style = document.createElement('style') - style.innerHTML = '.GitHubTip-actionButton :hover { color: #FB542B }' + style.appendChild( + document.createTextNode('.GitHubTip-actionButton :hover { color: #FB542B }') + ) shadowRoot.appendChild(style) return braveTipAction @@ -123,7 +125,7 @@ const getCommentMetaData = (elem: Element): RewardsTip.MediaMetaData | null => { const authorCollection = ancestor.getElementsByClassName('author') if (authorCollection.length) { const author = authorCollection[0] as HTMLElement - const userName = author.innerHTML + const userName = author.textContent return { mediaType: 'github', userName: userName || '' @@ -155,7 +157,7 @@ const getReviewItemMetaData = (elem: Element): RewardsTip.MediaMetaData | null = const authorCollection = ancestor.getElementsByClassName('author') if (authorCollection.length) { const author = authorCollection[0] as HTMLElement - const userName = author.innerHTML + const userName = author.textContent return { mediaType: 'github', userName: userName || '' @@ -254,7 +256,7 @@ const getPageHeadMetaData = (elem: Element): RewardsTip.MediaMetaData | null => const aTags = author.getElementsByTagName('A') if (aTags.length) { const aTag = aTags[0] as HTMLAnchorElement - const userName = aTag.innerHTML + const userName = aTag.textContent return { mediaType: 'github', userName: userName || '' diff --git a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/reddit.ts b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/reddit.ts index e767111dba5f..50f7ab3ad8ac 100644 --- a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/reddit.ts +++ b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/reddit.ts @@ -148,7 +148,8 @@ const createRedditTipButton = () => { braveTipButton.type = 'button' const style = document.createElement('style') - style.innerHTML = '.reddit-actionButton :hover { color: #FB542B }' + const css = '.reddit-actionButton :hover { color: #FB542B }' + style.appendChild(document.createTextNode(css)) braveTipButton.appendChild(style) return braveTipButton @@ -232,13 +233,15 @@ const createRedditTipActionCountPresentation = () => { const createHoverStyleElement = (isPost: boolean) => { // Create style element for hover const style = document.createElement('style') - style.innerHTML = isPost ? ':host { outline: none } :host(:hover) { background-color: var(--newRedditTheme-navIconFaded10) }' : '.reddit-actionButton { text-decoration: none; color: var(--newCommunityTheme-actionIcon); font-weight: bold; padding: 0px 1px; } .reddit-actionButton:hover { color: var(--newCommunityTheme-bodyText); text-decoration: underline }' + const css = isPost ? ':host { outline: none } :host(:hover) { background-color: var(--newRedditTheme-navIconFaded10) }' : '.reddit-actionButton { text-decoration: none; color: var(--newCommunityTheme-actionIcon); font-weight: bold; padding: 0px 1px; } .reddit-actionButton:hover { color: var(--newCommunityTheme-bodyText); text-decoration: underline }' + style.appendChild(document.createTextNode(css)) return style } const createHoverStyleElementForOld = () => { const style = document.createElement('style') - style.innerHTML = '.reddit-actionButton { color: #888; font-weight: bold; paddings: 0 1px; text-decoration: none } .reddit-actionButton:hover { text-decoration: underline }' + const css = '.reddit-actionButton { color: #888; font-weight: bold; paddings: 0 1px; text-decoration: none } .reddit-actionButton:hover { text-decoration: underline }' + style.appendChild(document.createTextNode(css)) return style } diff --git a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts index 5855782f25be..e211f272156f 100644 --- a/components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts +++ b/components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts @@ -252,7 +252,8 @@ const createBraveTipAction = (tweet: Element, tweetId: string, numActions: numbe // Create style element for hover color const style = document.createElement('style') - style.innerHTML = '.ProfileTweet-actionButton :hover { color: #FB542B }' + const css = '.ProfileTweet-actionButton :hover { color: #FB542B }' + style.appendChild(document.createTextNode(css)) shadowRoot.appendChild(style) return braveTipAction diff --git a/components/flags_ui/resources/brave_flags_overrides.js b/components/flags_ui/resources/brave_flags_overrides.js index 35f13c8480d8..ce170ea81da9 100644 --- a/components/flags_ui/resources/brave_flags_overrides.js +++ b/components/flags_ui/resources/brave_flags_overrides.js @@ -3,13 +3,21 @@ * 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/. */ -document.addEventListener('DOMContentLoaded', function() { - $('channel-promo-beta').innerHTML = ` - Interested in cool new Brave features? Try our - beta channel. - ` - $('channel-promo-dev').innerHTML = ` - Interested in cool new Brave features? Try our - nightly channel. - ` +document.addEventListener('DOMContentLoaded', function () { + [$('channel-promo-beta'), $('channel-promo-dev')].forEach((node, index) => { + const text1 = document.createTextNode('Interested in cool new Brave features? Try our ') + const text2 = document.createTextNode('.') + const link = document.createElement('a') + node.textContent = '' + node.appendChild(text1) + node.appendChild(link) + node.appendChild(text2) + if (index === 0) { + link.setAttribute('href', 'https://brave.com/download-beta/') + link.textContent = 'beta channel' + } else { + link.setAttribute('href', 'https://brave.com/download-nightly/') + link.textContent = 'nightly channel' + } + }) }); diff --git a/package.json b/package.json index 4e50f652f609..612d8a462dfb 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "main": "index.js", "scripts": { "audit_deps": "node ./build/commands/scripts/audit.js", + "check_security": "node ./build/commands/scripts/checkSecurity.js", "cibuild": "node ./build/commands/scripts/commands.js cibuild", "init": "npm --prefix ../../ run init --", "delete_string_translations": "node ./build/commands/scripts/commands.js delete_string_translations", @@ -25,7 +26,7 @@ "lint": "node ./build/commands/scripts/commands.js lint", "test": "node ./build/commands/scripts/commands.js test", "test:scripts": "jest build/commands/lib build/commands/scripts", - "test-security": "npm run audit_deps && node ./build/commands/scripts/commands.js start --enable_brave_update --network_log --user_data_dir_name=brave-network-test", + "test-security": "npm run check_security && npm run audit_deps && node ./build/commands/scripts/commands.js start --enable_brave_update --network_log --user_data_dir_name=brave-network-test", "tslint": "tslint --project tsconfig-lint.json \"components/**/*.{ts,tsx}\"", "pep8": "pycodestyle --max-line-length 120 -r script", "web-ui-gen-grd": "node components/webpack/gen-webpack-grd", diff --git a/ui/webui/resources/polymer_overriding.js b/ui/webui/resources/polymer_overriding.js index 81a842df40b8..0b598aea1d32 100644 --- a/ui/webui/resources/polymer_overriding.js +++ b/ui/webui/resources/polymer_overriding.js @@ -184,7 +184,12 @@ export function OverrideIronIcons(iconSetName, overridingIconSetName, iconOverri continue } // replace - chromiumIcon.innerHTML = braveIcon.innerHTML + while (chromiumIcon.firstChild) { + chromiumIcon.firstChild.remove() + } + while (braveIcon.firstChild) { + chromiumIcon.appendChild(braveIcon.firstChild) + } } // Ensure icons get re-parsed if already parseds // `getIconNames` ensures this._icons in iron-iconset-svg is re-parsed @@ -240,4 +245,4 @@ Polymer.Class = function (info, mixin) { // Overrides for all pages RegisterStyleOverride('cr-toggle', CrToggleStyleTemplate) -RegisterStyleOverride('cr-button', CrButtonStyleTemplate) \ No newline at end of file +RegisterStyleOverride('cr-button', CrButtonStyleTemplate)