-
-
Notifications
You must be signed in to change notification settings - Fork 441
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
Arbitrary Code Execution via JavaScript Queries (CVE-2021-42057) #615
Comments
Holy crap lol. Didn't realize it was that easy to run pure node functions. Could definitely be deadly. |
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 |
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. |
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. |
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
|
@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 The README notice is a good idea - I will add that as a disclaimer in the README. |
@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 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. |
@blacksmithgu |
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. |
I am aware of the risks of evaluating code one does not understand. 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: |
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. |
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. |
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? |
That's a good point. 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. |
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):
|
I like the idea of marking a block as safe the first time it is executed. |
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? |
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: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.
The text was updated successfully, but these errors were encountered: