diff --git a/docs/tutorial/security.md b/docs/tutorial/security.md index f16d8cf49bc9..876ed8baade3 100644 --- a/docs/tutorial/security.md +++ b/docs/tutorial/security.md @@ -36,6 +36,7 @@ things on top of Electron. Pull requests and contributions supporting this effort are always very welcome. ## Ignoring Above Advice + A security issue exists whenever you receive code from a remote destination and execute it locally. As an example, consider a remote website being displayed inside a browser window. If an attacker somehow manages to change said content @@ -49,6 +50,7 @@ your application) to execute Node code. To display remote content, use the `webview` tag and make sure to disable the `nodeIntegration`. #### Checklist + This is not bulletproof, but at the least, you should attempt the following: * Only display secure (https) content @@ -71,3 +73,22 @@ This is not bulletproof, but at the least, you should attempt the following: Again, this list merely minimizes the risk, it does not remove it. If your goal is to display a website, a browser will be a more secure option. + +## Buffer Global + +Node's [Buffer](https://nodejs.org/api/buffer.html) class is currently available +as a global even when `nodeIntegration` is set to `false`. You can delete +this in your app by doing the following in your `preload` script: + +```js +delete global.Buffer +``` + +Deleting it may break Node modules used in your preload script and app since +many libraries expect it to be a global instead of requiring it directly via: + +```js +const {Buffer} = require('buffer') +``` + +The `Buffer` global may be removed in future major versions of Electron. diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index 96f750897d09..2088f39b37e7 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -2,6 +2,7 @@ const {app, ipcMain, webContents, BrowserWindow} = require('electron') const {getAllWebContents} = process.atomBinding('web_contents') const renderProcessPreferences = process.atomBinding('render_process_preferences').forAllWebContents() +const {Buffer} = require('buffer') const fs = require('fs') const path = require('path') const url = require('url') diff --git a/lib/browser/init.js b/lib/browser/init.js index 6ff328830e59..fb1688d0cc74 100644 --- a/lib/browser/init.js +++ b/lib/browser/init.js @@ -1,5 +1,6 @@ 'use strict' +const {Buffer} = require('buffer') const fs = require('fs') const path = require('path') const util = require('util') diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index ed8b29add691..31e5a37732c7 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -1,5 +1,6 @@ 'use strict' +const {Buffer} = require('buffer') const electron = require('electron') const v8Util = process.atomBinding('v8_util') const {ipcMain, isPromise, webContents} = electron diff --git a/lib/common/asar.js b/lib/common/asar.js index 40fdec9494ee..230e3113daab 100644 --- a/lib/common/asar.js +++ b/lib/common/asar.js @@ -1,5 +1,6 @@ (function () { const asar = process.binding('atom_common_asar') + const {Buffer} = require('buffer') const childProcess = require('child_process') const path = require('path') const util = require('util') diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 7f7672327a45..a3a4443413d3 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -1,5 +1,6 @@ 'use strict' +const {Buffer} = require('buffer') const v8Util = process.atomBinding('v8_util') const {ipcRenderer, isPromise, CallbacksRegistry} = require('electron') diff --git a/lib/renderer/init.js b/lib/renderer/init.js index 710c4c7c2db7..2356a81dda2e 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -123,7 +123,7 @@ if (nodeIntegration === 'true') { delete global.process delete global.setImmediate delete global.clearImmediate - return delete global.global + delete global.global }) } diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ba8c979a9f8b..d86bfe228cb0 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -542,6 +542,22 @@ describe('browser-window module', function () { }) w.loadURL('file://' + path.join(fixtures, 'api', 'preload.html')) }) + + it('can successfully delete the Buffer global', function (done) { + var preload = path.join(fixtures, 'module', 'delete-buffer.js') + ipcMain.once('answer', function (event, test) { + assert.equal(test.toString(), 'buffer') + done() + }) + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + preload: preload + } + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'preload.html')) + }) }) describe('"node-integration" option', function () { diff --git a/spec/fixtures/module/delete-buffer.js b/spec/fixtures/module/delete-buffer.js new file mode 100644 index 000000000000..d88e8b57708d --- /dev/null +++ b/spec/fixtures/module/delete-buffer.js @@ -0,0 +1,11 @@ +const path = require('path') +const {remote} = require('electron') +const {Buffer} = window + +delete window.Buffer +delete global.Buffer + +// Test that remote.js doesn't use Buffer global +remote.require(path.join(__dirname, 'print_name.js')).echo(new Buffer('bar')) + +window.test = new Buffer('buffer')