add electronegativity #1202

goosewobbler · 2022-11-26T03:07:22Z

Running Electronegativity on PR builds

Warnings

One or more CSP directives detected are vulnerable
line 4 in dapp/dapp.html

'self' can be problematic if you host JSONP, Angular or user uploaded files
- we host none of these, but should remove default-src in favour of more strict rules
- Replace default-src with script-src 'strict-dynamic' 'nonce-<nonce>'; base-uri 'self';
Allow only resources downloaded over HTTPS. - 'img-src'
- Local dapp server required to run over https. If a user's machine is compromised to the extent where localhost can be accessed there will be worse problems. Ignored.
Consider requiring Trusted Types for scripts to lock down DOM XSS injection sinks. You can do this by adding "require-trusted-types-for 'script'" to your policy.
- add require-trusted-types-for

Missing .on new-window navigation limit
file:///

False positive for LIMIT_NAVIGATION_GLOBAL_CHECK - doyensec/electronegativity#92 (comment) - Excluded in GA file

Search for dangerous runtime flags in the package.json file.
line 31 in package.json
line 36 in package.json

Complaining about node --inspect in dev scripts, ignored

Limit navigation flows to untrusted origins. Middle-click may cause Electron to open a link within a new window
Review the use of the contextIsolation option
Use sandbox for untrusted origins
line 110 in main/windows/index.ts
line 97 in main/windows/extractColors/index.ts
line 34 in main/windows/frames/frameInstances.ts
line 28 in main/windows/frames/viewInstances.ts

EN unable to directly link these options to where the BrowserWindows are initialised. Fixed by extracting BrowserWindow and BrowserView creation:

extract BrowserWindow in main/windows/index.ts
extract BrowserWindow in main/windows/extractColors/index.ts
extract BrowserView in main/windows/extractColors/index.ts
extract BrowserWindow in main/frames/frameInstances.ts
extract BrowserView in main/windows/frames/viewInstances.ts

Notes

One or more CSP directives detected seems to be vulnerable
line 4 in flow/flow.html
line 4 in dash/dash.html
line 4 in app/tray.html

Same as for dapp/dapp.html above.

Review the use of openExternal
line 201 in main/index.ts
line 210 in main/index.ts
line 69 in main/updater/index.ts
line 289 in main/accounts/index.ts

Opening external links is limited to block explorer links, release pages for our updates and some white listed external assets. Can ignore, but we can condense the above notes by ensuring external links are only opened from a single file:

Open external links in one place

Review the use of preload scripts
line 99 in main/windows/extractColors/index.ts
line 47 in main/windows/frames/frameInstances.ts
line 30 in main/windows/frames/viewInstances.ts

We need preload scripts for the app to function. Now only set in the main/windows/window.ts file with the BrowserWindow / BrowserView init.

Set preload scripts in one place

TODO:

Address or ignore all raised warnings & notes
Verify production build
Post-merge: Rebase deps and check EN output there
- address or ignore anything new

main/windows/index.ts

main/windows/frames/frameInstances.ts

main/windows/window.ts

+    skipTaskbar: process.platform !== 'linux',
+    webPreferences: {
+      ...webPreferences,
+      preload: path.resolve(process.env.BUNDLE_LOCATION, 'bridge.js'),


main/windows/window.ts

+      scrollBounce: true,
+      navigateOnDragDrop: false,
+      disableBlinkFeatures: 'Auxclick',
+      preload: path.resolve('./main/windows/viewPreload.js'),


main/windows/window.ts

+    }
+  })
+
+  viewInstance.webContents.on('will-navigate', (e) => e.preventDefault())


…into electronegativity

dapp/dapp.html

flow/flow.html

dash/dash.html

app/tray.html

dapp/dapp.html

@@ -1,12 +1,24 @@
 <html>
  <head>
    <meta charset='utf-8' />
-    <meta http-equiv='Content-Security-Policy' content="default-src 'self'; connect-src *; img-src blob: http://localhost:8421; style-src 'self' 'unsafe-inline'; frame-src 'none'; object-src 'none';"/>
+    <meta 


main/windows/window.ts

+export function openExternal(url = '') {
+  if (isWhitelistedHost(url) || isValidReleasePage(url)) {
+    store.setDash({ showing: false })
+    shell.openExternal(url)


main/windows/window.ts

+  browserWindow.webContents.once('did-finish-load', () => {
+    log.info(`Created ${name} renderer process, pid:`, browserWindow.webContents.getOSProcessId())
+  })
+  browserWindow.webContents.on('will-navigate', (e) => e.preventDefault()) // Prevent navigation


mholtzman

lgtm, just one minor issue

.github/workflows/compile-and-test.yml

main/windows/window.ts

dash/App/Notify/index.js

goosewobbler added 3 commits November 26, 2022 03:04

add electronegativity

9be5c5b

remove whitespace

e9acd7f

update install

c0a8cfa

goosewobbler added tooling security labels Nov 26, 2022

goosewobbler and others added 2 commits November 26, 2022 03:27

Update compile-and-test.yml

b771c22

update versions

21e779c

goosewobbler added the WIP PRs that are still in progress and not ready for review or merging label Nov 26, 2022

goosewobbler changed the base branch from develop to deps November 26, 2022 22:27

goosewobbler changed the base branch from deps to develop November 26, 2022 22:28

improve use of webPreferences

a139539

github-advanced-security bot found potential problems Nov 27, 2022

View reviewed changes

main/windows/index.ts Fixed Show fixed Hide fixed

main/windows/index.ts Fixed Show fixed Hide fixed

main/windows/index.ts Fixed Show fixed Hide fixed

main/windows/frames/frameInstances.ts Fixed Show fixed Hide fixed

goosewobbler added 2 commits November 27, 2022 19:28

use spread

d067936

extract window & view creation

b97f44f

github-advanced-security bot found potential problems Nov 27, 2022

View reviewed changes

goosewobbler added 3 commits November 27, 2022 21:38

Update compile-and-test.yml

74a6d8a

Update compile-and-test.yml

7d53679

Update compile-and-test.yml

cd8a613

github-advanced-security bot found potential problems Nov 27, 2022

View reviewed changes

goosewobbler and others added 3 commits November 28, 2022 14:40

Update window.ts

44d8519

move over rules / formatting from deps

a0a430f

Merge branch 'electronegativity' of https://github.com/floating/frame …

61592b0

…into electronegativity

github-advanced-security bot found potential problems Nov 28, 2022

View reviewed changes

dapp/dapp.html Fixed Show fixed Hide fixed

flow/flow.html Fixed Show fixed Hide fixed

dash/dash.html Fixed Show fixed Hide fixed

app/tray.html Fixed Show fixed Hide fixed

goosewobbler added 2 commits November 28, 2022 18:07

add require-trusted-types-for 'script';

9b9cb5b

stricter CSPs

6ca15e7

github-advanced-security bot found potential problems Nov 28, 2022

View reviewed changes

goosewobbler added 2 commits November 28, 2022 23:07

fix extractColors error

e62701e

extract openExternal

3ab1fce

github-advanced-security bot found potential problems Nov 29, 2022

View reviewed changes

clean up

1a0de94

goosewobbler removed the WIP PRs that are still in progress and not ready for review or merging label Nov 29, 2022

move handlers to window

b551d89

github-advanced-security bot found potential problems Nov 29, 2022

View reviewed changes

explain global restriction of webContents

f368cce

mholtzman requested changes Nov 29, 2022

View reviewed changes

.github/workflows/compile-and-test.yml Outdated Show resolved Hide resolved

main/windows/window.ts Outdated Show resolved Hide resolved

goosewobbler added 2 commits November 29, 2022 15:32

closeDash on openExternal

693ab0a

use env for npm

72491d5

mholtzman reviewed Nov 29, 2022

View reviewed changes

dash/App/Notify/index.js Outdated Show resolved Hide resolved

goosewobbler added 2 commits November 29, 2022 15:42

ABORT

6d359aa

update

98064b5

mholtzman approved these changes Nov 29, 2022

View reviewed changes

goosewobbler merged commit db05852 into develop Nov 29, 2022

goosewobbler deleted the electronegativity branch November 29, 2022 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add electronegativity #1202

add electronegativity #1202

goosewobbler commented Nov 26, 2022 •

edited

Loading

mholtzman left a comment

add electronegativity #1202

add electronegativity #1202

Conversation

goosewobbler commented Nov 26, 2022 • edited Loading

Warnings

Notes

mholtzman left a comment

Choose a reason for hiding this comment

goosewobbler commented Nov 26, 2022 •

edited

Loading