Merge pull request #7142 from electron/require-buffer

Require Buffer explicitly instead of relying on global

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.

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')

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')

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

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')

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')
 

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
   })
 }
 

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 () {

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')