Skip to content
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

sanitize link-click event that uses shell.openExternal function #51

Closed
3 tasks
cloverich opened this issue Sep 22, 2021 · 1 comment · Fixed by #153
Closed
3 tasks

sanitize link-click event that uses shell.openExternal function #51

cloverich opened this issue Sep 22, 2021 · 1 comment · Fixed by #153

Comments

@cloverich
Copy link
Owner

Related: #297

See this Electron explanation

To allow users to click a link rendered in markdown, and have it open an external browser, I provde this functionality:

// in renderer process:
function externalLinksListener(event: any) {
  // hopefully only relevent on <a href="..."> elements
  if (event.target.tagName !== 'A') return;

  const link = event.target.getAttribute('href');
  // don't navigate the main window to an external url
  event.preventDefault();

  // if its not a reference link, open an external browser:
  if (link && link.indexOf('#') !== 0) {
    ipcRenderer.send('link-click', link);
  }
}


// in main process:

// Open browser windows on link-click, an event triggered by renderer process.
ipcMain.on('link-click', (event: any, link: string) => {
  shell.openExternal(link);
});

This works but presents a security issue: if the content is untrusted, the openExternal call could open any program, even a terminal: RCE. Some thoughts:

  • allow user to optionally disable this behavior
  • validate that the link is a valid web url; disable opening any other kind of application
  • ???

I decided to leave it enabled for now, because to work it would at least require the user to copy and paste a malicious link. If I allow html (etc.) in the editor, technically it could also be a script that fires a link click event.

@cloverich
Copy link
Owner Author

it would at least require the user to copy and paste a malicious link. If I allow html (etc.) in the editor, technically it could also be a script that fires a link click event.

Also any of the included dependencies (transitive) could trigger this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant