obsidian-dataview icon indicating copy to clipboard operation
obsidian-dataview copied to clipboard

Arbitrary Code Execution via JavaScript Queries (CVE-2021-42057)

Open tivey-scwx opened this issue 2 years ago • 17 comments

Describe the bug

I discovered a way to craft malicious markdown files that will cause the obsidian-dataview plugin to execute arbitrary commands on users’ systems. This is due to the unsafe use of eval within the evalInContext function located in src/api/inline-api.ts.

This has been assigned a CVE of CVE-2021-42057 for tracking.

To Reproduce

The following proof-of-concept can be used to display a file on a user’s system by executing the cat command:

```dataviewjs
require("child_process").exec("cat /etc/passwd",(_0,stdout,_1) => dv.span(stdout));""
```

A malicious user could leverage this vulnerability to execute arbitrary code on other users' systems by getting them to open an untrusted markdown file. This is especially dangerous in environments where users share vaults.

Expected behavior

DataviewJS should not make an unsafe call to eval using user supplied input.

Additional Context

Shortly after we privately disclosed this issue, @blacksmithgu promptly changed the default behavior of Dataview to no longer enable JavaScript Queries by default (see release 0.4.13). This helps protect new dataview users and provides a way for existing dataview users to mitigate this issue by disabling the JavaScript Query functionality when opening untrusted markdown.

@blacksmithgu is currently working on additional solutions and provided permission for us to open a public issue here for tracking.

tivey-scwx avatar Nov 04 '21 19:11 tivey-scwx

Holy crap lol. Didn't realize it was that easy to run pure node functions. Could definitely be deadly.

KjellConnelly avatar Nov 04 '21 23:11 KjellConnelly

DataviewJS blocks run with the same permissions that Dataview-the-plugin has, which apparently is much more than I initially expected - you can access node functions and various other libraries which plugins probably shouldn't have access too!

I am looking at improved sandboxing for DataviewJS blocks via the vm2 library, which will incur a performance overhead and block all external require calls (except through possibly a whitelisted list of modules / other user scripts). Another alternative would be for Obsidian to prevent plugins from using these scary APIs in the first place.

blacksmithgu avatar Nov 06 '21 00:11 blacksmithgu

As for risk, the main takeaway is: be careful with what DataviewJS code you copy-paste from the internet into your vault. Don't run minified code or code that looks like gibberish, and make sure any shared code has been vetted by others (or you understand it yourself).

The main risk is through requiring node packages or making suspicious HTTP calls (through the Obsidian API) to URLs you don't recognized.

blacksmithgu avatar Nov 06 '21 00:11 blacksmithgu

From my perspective js execution is a significant feature that I don't want to see removed.

I do not agree that arbitration execution of dataview queries within trusted markdown code is a security vulnerability.

Downloading a bash script or any from the internet is not in itself harmful as it wont do anything until you execute it. And you should only execute code from trusted sources.

The question then becomes do I trust the markdown files in my vault.

Yes, I wrote them and if I find a snippet of obsidian markdown online containing dataviewjs I am certainly going to review it before copying it into my vault.

However, it is not generally expected that markdown files contain executable code and thus I fully agree that it should be disabled by default.

The user has already acknowledged that enabling plugins allows arbitrary code execution and starts with plugins disabled.

The plugin also requires enabling js explicitly.

Could more be done? perhaps. It might be a good idea to create a .dataview/trusted file with list of markdown files. If dataview detects js in a file not in this list it could prompt the user.

But I personally don't want to be prompted every time I generate a new markdown file from my daily notes template.

Enabling this feature does require the user to be mindful of the code he adds to his vault but that in itself does not make it a bug.

I also do not want the js to be "sandboxed" in fact I want to call http endpoints to import tasks from jira and execute a shell script to pull in my google calendar.

I could potentially make these into plugins, but I love the ability to use dataviewjs to prototype these things.

kurtharriger avatar Nov 18 '21 12:11 kurtharriger

I agree with @kurtharriger - arbitrary code should not be protected. The only caviat I would say is that if you did want to address this issue, it should be in the README.md. Something basically saying,

Warning! As with all community Obsidian plugins, this plugin uses third party code. Due to the nature of this plugin, you as the user of the plugin must be aware of the new level of potential arbitrary code being run with dataviewjs. Be careful to understand the code you run, as code run is not sandboxed. This means that if you run malicious code (hypothetically by copying/pasting), not only could your vault be compromised, but any data on your computer may be as well.

KjellConnelly avatar Nov 18 '21 18:11 KjellConnelly

@kurtharriger @KjellConnelly I agree with both of your points. To be clear, I am absolutely not removing JS functionality (I use it for everything personally), and I am also trying to avoid annoying security popups since people will just always press "Yes" on them anyway by the time they reach them (if you don't trust the code, you wouldn't copy it into your vault in the first place!).

The sandboxing will be toggleable with a "I know what I'm doing"-type setting that turns it off; it will still allow AJAX and full use of Obsidian/Dataview APIs, it just blocks node library requires() by default.

The README notice is a good idea - I will add that as a disclaimer in the README.

blacksmithgu avatar Nov 18 '21 20:11 blacksmithgu

@blacksmithgu Great idea with just blocking requires. I personally hate how difficult it is to deal with permissions sometimes. It can get quite convoluted. And require() is basically the tip of the iceberg when it comes to node. So beginners won't have too much trouble understanding how to use node vs much more.

By the way, just to let you know, developers can access many node features directly from window.app within obsidian. For instance, fs is one module I use without require. I don't know if it's a full port or a wrapper or what, but I do use it to generate non .md files.

KjellConnelly avatar Nov 18 '21 22:11 KjellConnelly

@blacksmithgu There hasn't been an update on this issue in a while. Is there any hope for preventing dataviewjs queries from using internal nodejs modules?

lkarbownik avatar Jul 13 '22 10:07 lkarbownik

I don't understand this is an issue. You are in complete control of the documents you place in your obsidian folder.

If you don't know what a script does don’t use it.

I recently switched to using Logseq instead of Obsidian. Logseq is also an electron app, but it does run scripts in a sandboxed environment with no option to elevate privileges.

This was a huge issue for me when I tried to develop a plugin that would query things3 database

https://discuss.logseq.com/t/privileged-plugins/4746

I eventually came up with a workaround where the user could install an additional app that runs as in background listening on HTTP port as an api server to enable queries from logseq maybe its better this way and maybe its not as having an app listening on http port exposes new venerabilities that did not exist when queried directly from electron. And it is certainly more complex for both the user and the developer.

https://discuss.logseq.com/t/things-3-plugin-proof-of-concept-and-feedback/4821

Having the option to sandbox scripts might be nice option to have, and logseq might provide a template on how it can be done. However it is not trivial and may significantly limit its capabilities requiring workaround like mine to access the filesystem for legitimate use cases.

I think most users of dataviewjs have sufficient knowledge of js to evaluate the code the choose to run and if they don't care about what the code they run then they shouldn't install or blame the plugin for their laziness any more than one should blame node itself for its ability to run arbitrary code.

With great power comes great responsibility.

kurtharriger avatar Jul 13 '22 14:07 kurtharriger

@kurtharriger

I am aware of the risks of evaluating code one does not understand. Luckily, I'm in a position, where I can (and will) do that. However, there are other attack vectors, such as adding content using a share sheet into your Obsidian vault. There are ways to hide/obscure malicious code when rendering HTML. Since we are getting more applications with dedicated Obsidian integration, there is a non-trivial chance, that someone with knowledge of this vulnerability will decide to target it and use it.

I understand the complexity of this issue and I would be the last one to suggest removing dataviewjs queries. It's an amazing and essential tool in my workflow. I did not intend to convey the "Why is this still not fixed?!" kind of message (and I hope I did not do that accidentally). @blacksmithgu did amazing work by building this plugin and by providing some layers of protection against malicious attacks. Since the issue is still in status "Open" and @blacksmithgu himself/herself mentioned that he/she is looking into options to sandbox the code evaluation I do not think that asking for a status update is an unreasonable thing to do.

edit: I do appreciate you sharing your experience with developing a Logseq integration.

lkarbownik avatar Jul 13 '22 16:07 lkarbownik

Fair enough.

I do agree that sharing documents and workspaces introduces some legitimate security risks, and a feature to sandbox js could be very valuable feature in that context.

But I disagree with the classification that this is a bug that needs to or should be fixed -- if that means removing the ability to execute arbitrary code entirely.

kurtharriger avatar Jul 13 '22 17:07 kurtharriger

Sure, I don't think I suggested that this is the case and I'm pretty sure we are on the same page.

I am only hoping to establish if this is something that is still being worked on, or whether we should accept this risk and include this risk factor long-term in our personal workflows.

lkarbownik avatar Jul 13 '22 18:07 lkarbownik

Doesn't this carry the same risk as any other Obs plugin? I'm rather poor on security fundamentals but I imagine several other plugins like Templater and CustomJS would have a similar if not identical CVE. Can anyone educate me?

AB1908 avatar Jul 13 '22 22:07 AB1908

That's a good point. I checked and Templater also has access to Node internals in the JavaScript template directive as well. I'm not familiar with CustomJS plugin.

I guess the main takeaway here is that if we're going to use these plugins, we need to be careful about the kind of content we add to Obsidian programmatically. This includes integrations (Readwise, PDF highlight extractors) and even native share functionalities like IOS/Android share buttons.

lkarbownik avatar Jul 13 '22 23:07 lkarbownik

I think the vulnerability is much more significant in Dataview than it is in Templater, since Templater only executes JS on file creation (at which point the file only contains whatever was in the template) or by the user deliberately running the Templater command on an existing file, whereas Dataview can execute JS from an untrusted source without the user intending to execute JS or even use Dataview in that file.

Let's take an attack vector that @lkarbownik mentioned for example: It's pretty trivial to create HTML that displays some completely normal-looking text, but on copy, copies a malicious script that is wrapped in a dataviewjs block to the user's clipboard. If an unsuspecting user were to copy that seemingly innocuous text and then paste it into their Obsidian vault, if they have DataviewJS enabled, the malicious code would execute. They may quickly realize that the pasted text does not match the text that they copied, but by then the code has already executed. For this to happen with Templater, the user would have to not notice that the pasted text doesn't match, and then manually run Templater on that file.

I love the power of Dataview (thanks @blacksmithgu!!) and definitely don't want arbitrary JS execution removed from it, but I also don't think just telling users to be careful with what they paste is sufficient, as it requires what I think is an unreasonable amount of care to guard against vulnerabilities like the one mentioned above. The careful approach would be to either completely avoid pasting external content into Obsidian or to always first paste any text into a plain text editor and manually check it before then pasting it into Obsidian. Most users will not be willing to go to these lengths, which leaves them exposed.

One simple mitigation for the specific issue mentioned above is to allow users to customize the keyword used to begin a dataviewjs block (I've done this on my local copy by changing the keyword here).

Other rough ideas (with no claims about feasibility):

  • Allow user to enable/disable DataviewJS via frontmatter or based on file path. That way users only need to practice extreme caution when they are in those files/directories (and can also mark those files as dangerous by styling them differently with CSS).
  • Limit JS execution to code in a specific directory. Code from other directories can reference the functions defined in these other files.
  • Allow DataviewJS everywhere but detect paste (is this possible?) and disable DataviewJS for blocks that were pasted in until the user marks them as safe.

eyuelt avatar Sep 22 '22 06:09 eyuelt

I like the idea of marking a block as safe the first time it is executed.
Potentially could hash the content and save hash of user approved scripts in the plugin config file.
This could get annoying if one is trying to write a template and making frequent changes, so perhaps also an option to temporarily execute all scripts, and while disabled display a banner at top of page warning that dataviewjs script authorization is currently disabled, click here to reenable.

kurtharriger avatar Sep 22 '22 07:09 kurtharriger

Along the lines of @eyuelt's example, imagining a website with a "copy to clipboard" button for anything (or maybe even automatically copying something to clipboard if possible), especially something like one of those rip-off stack overflow aggregators. Something like that where people would often copy some info and paste it into obsidian.

Also considering in shared vault, your "friend/colleague" temporarily enabling JS execution, running some remote commands on your system, and then disabling JS execution again unbeknownst to you. But I guess in that sense, couldn't anyone add arbitrary javascript to any plugin being shared/synced in the .obsidian folder to cause havoc? Then essentially any shared vault opens up arbitrary code execution?

digitalsignalperson avatar Sep 30 '22 23:09 digitalsignalperson