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

Remote code execution vulnerability #384

Open
theGEBIRGE opened this issue May 7, 2024 · 4 comments
Open

Remote code execution vulnerability #384

theGEBIRGE opened this issue May 7, 2024 · 4 comments

Comments

@theGEBIRGE
Copy link

Hey, I've discovered a vulnerability in obsidian-annotator. I'm sticking to GitHub's default template for advisories (maybe consider adding a SECURITY.md):

Summary

Opening an ebook with malicious scripts inside can lead to remote code execution on the users's machine.
Testing was done with Obsidian 1.5.12 and Annotator 0.2.11 on Windows.

Details

Because of the epub.js configuration option allowScriptedContent = true, it is possible to execute arbitrary JavaScript code from within an epub file:

allowScriptedContent: true

epub.js itself uses an iframe to display the epubs. While it does set the sandbox attribute, it also sets allow-same-origin. This can't be changed by the consumer of the library. A combination of allow-scripts and allow-same-origin renders the sandboxing obsolete (see here).

The developers of epub.js warn about this.

In the case of obsidian-annotator, exploitation is really simple, as the attacker has access to the node integration.

PoC

An ebook can be crafted with Calibre to include this bare minimum script:

window.top.require("child_process").execSync("calc");

That's it!

Impact

Users have to point the annotation-target to a malicious file (be that on the local or a remote system) and start to annotate it.
However, the attacker doesn't have to prepare a book specifically for obsidian-annotator, but can use some fingerprinting to determine in what environment it's running.

Distribution of malicious books could be done via pirate sites or even (online) conversion services, which could inject those malicious scripts.

Some ideas

In an ideal world, scripted content would be turned off. There are, however, limitations with that approach.
The author of foliate sums it up nicely here.
Maybe the user could be given the option to toggle scripted content.

That's it! If something's unclear, please ask away.

Cheers
Frederic

PS: Audio warning for the PoC video!

obsidian-annotator-rce-poc.mp4
@elias-sundqvist
Copy link
Owner

Nice investigation 👍

Ideally people will just load trusted sources, but I can imagine pirated epubs being an issue...

A short term solution could maybe be to show a warning message to users when they try to load epubs?

Longer term it would be good if we could properly block such script execution though.

@theGEBIRGE
Copy link
Author

Thanks! :^)

That sounds like a good solution.

I don't know how many ebooks are relying on script execution to present their content. Probably not too many.
As far as I can tell, the main reason for enabling scripts is this WebKit bug that prevents events from propagating into the main window (touch events etc.).

If that's a usability trade-off that you're willing to make, you could turn scripted content off by default and let the user decide when to enable it.

@johnfactotum
Copy link

For now, blocking scripts with CSP, if possible, would be a better option than the Epub.js setting, as the WebKit bug would break stuff like showing a popover on selection change, which kind of an essential feature for all reading systems that allow annotations. Another advantage of CSP is that it allows you to block stuff like remote resources, which could be a privacy concern. Alternatively, blocking at the WebView level would be preferable if possible.

And—this is really an edge case, but there is another reason why you might want to block scripts even if you trust that the EPUB file is not malicious. Some EPUBs use APIs like localStorage to persist their own data.1 This could result in conflict and data loss if you don't serve different2 EPUBs from different origins, which is difficult and most reading systems probably don't. In fact—I'm not familiar with Obsidian, but if plugins are allowed to access localStorage and they all run in the same origin, that would extend the problem to the plugin level.

Footnotes

  1. localStorage is in fact supported by iBooks, according to https://stackoverflow.com/questions/18348214/does-ibooks-support-localstorage-in-the-epub-format

  2. To make matters worse, there's no real definition as to what counts as "the same EPUB", so depending on how you handle that, e.g. if you're using a weak hash to identify EPUB files, it might possible for a malicious EPUB to impersonate another EPUB and read its data.

@theGEBIRGE
Copy link
Author

theGEBIRGE commented May 16, 2024

Thanks for the clarification and detailed explanation.
I don't know how restrictive plugin development for Obsidian is, but I imagine one has the ability to specify a meta tag in order to implement a CSP.

johnfactotum added a commit to johnfactotum/epub-test that referenced this issue May 17, 2024
See elias-sundqvist/obsidian-annotator#384

Note that in recent versions of Electron, for this to work, one must set
`nodeIntegration: true` and `contextIsolation: false` when constructing
the `BrowserWindow`.
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

No branches or pull requests

3 participants