-
-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Security Best Practices (Help Wanted) #742
Comments
Issue: webPreferences.webSecurityWhen you try to display images or other files in a renderer window with Most answers on the internet tell you to just set I don't know how dangerous it actually is to use an app with disabled webSecurity, but according to the docs and to Electron maintainer @MarshallOfSound, disabling webSecurity is a bad idea:
SolutionTo make electron-builder v2.0 safer by default, and prevent developers from monkey-patching their apps with Main process:import { protocol } from 'electron'
...
app.on('ready', () => {
registerSafeFileProtocol()
...
})
function registerSafeFileProtocol() {
const safeFileProtocol = `${appName}-safe-file-protocol`
protocol.registerFileProtocol(safeFileProtocol, (request, callback) => {
const url = request.url.replace(`${safeFileProtocol}://`, '')
// Decode URL to prevent errors when loading filenames with UTF-8 chars or chars like "#"
const decodedUrl = decodeURIComponent(url)
try {
return callback(decodedUrl)
}
catch (error) {
console.error('ERROR: main | registerSafeFileProtocol | Could not get file path', error)
}
})
} I think it should add the app name to the protocol:
And then open a file from a URL using the protocol, e.g. DocsAlso docs would have to be updated to explain how to use the protocol to load files: Method 1: load image in a renderer window:Renderer process:<img :src="getSafePath('C:/test.jpg')"> computed: {
getSafePath (path) {
// return `${__safeFileProtocol}://${path}`
return `appName-safe-file-protocol://${path}`
}
} Method 2: load path selected by user via dialog from the main process:Main process:ipcMain.on('open-file-dialog', (event) => {
const window = BrowserWindow.getFocusedWindow()
dialog.showOpenDialog(window, { properties: ['openFile'] })
.then(result => {
// Send the path back to the renderer
event.sender.send('open-file-dialog-reply', { path: result.filePaths[0] })
})
.catch(error => {
console.log('ERROR: main | open-file-dialog | Could not get file path')
})
}) Renderer process:<img id="image-1"> mounted () {
ipcRenderer.on('open-file-dialog-reply', (event, data) => {
this.loadImage(data.path)
}
},
methods: {
loadImage (path) {
const image1 = document.getElementById('image-1')
image1.src = `appName-safe-file-protocol://${path}`
}
} |
Following comments from #610 someone made a electron template for react that targets security only. take a look at secure-electron-template). |
I have multiple pages that I load in BrowserWindow's. Some require nodeIntegration, some do not. It seems like the current patch for
Here's what I need:
|
I think the Safely Accessing Node APIsAlthough it's sometimes mentioned, the global The safer alternative is to use a preload file whereby you expose your own functions that use the Node API. Here's an example: // vue.config.js
module.exports = {
pluginOptions: {
electronBuilder: {
preload: 'src/preload.js',
// Or, for multiple preload files:
// preload: { preload: 'src/preload.js', otherPreload: 'src/preload2.js' }
}
}
} // src/background.js (short snippet)
// Create the browser window.
const win = new BrowserWindow({
webPreferences: {
// Use pluginOptions.nodeIntegration, leave this alone
// See nklayman.github.io/vue-cli-plugin-electron-builder/guide/security.html#node-integration for more info
// nodeIntegration: process.env.ELECTRON_NODE_INTEGRATION,
preload: __dirname + '/preload.js',
}
}) // src/preload.js
// In this file, we can call Node APIs and packages via require() without
// exposing our entire system to the renderer. We only expose specific functions.
// Remember to handle all user-input safely so that these functions cannot be abused
// against their original intention.
const fs = require('fs');
window.myFileSystemOperation = function() {
const folders = fs.readdirSync('/');
console.log(folders);
} With that code in place, we can now call |
The docs for nodeIntegration with ipcRenderer or contextBridge should be updated: While your example is OK for local rendering, there should be a disclaimer for loading remote urls, or use the "Good Code" example: |
@Sparkenstein / @aleksey-hoffman (Author of secure-electron-template here). I've written a blog post describing how one can load images in development and production environments (which is also secure) - in case this is something you are still looking answers for. |
I've recently seen a handful of security related suggestions such as #733 (comment) and #610 (comment). With v2.0 of this plugin I'd like to make it much more secure by default. I've started by disabling
nodeIntegration
by default, but I'd also like to add a security section to the docs.If you have any suggestions related to security, whether it be something I should change in the
background.js
template or something I should explain in this new section, please leave a comment below. As a bonus, I'd really appreciate a short tutorial on how to configure it if the process isn't as simple as changing a line or two of code. Thank you for your help making the many apps built with this plugin more secure.EDIT: The security page is now live, but I'll leave this open as I could still add more to it.
The text was updated successfully, but these errors were encountered: