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

(Bug report) Scripts can overwrite files in application install dir #492

Open
maphew opened this issue Oct 15, 2024 · 4 comments
Open

(Bug report) Scripts can overwrite files in application install dir #492

maphew opened this issue Oct 15, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@maphew
Copy link

maphew commented Oct 15, 2024

TriliumNext Version

0.90.7-beta

What operating system are you using?

Windows

What is your setup?

Local (no sync)

Operating System Version

win 11 pro (10.0.22631 N/A Build 22631)

Description

Scripts executed within Trilium have full write abilities to the Trilium application folder (on Windows, linux not tested).

Demonstration

Create a Code note of type JS Backend:

// export the current note and branch to zip file
api.exportSubtreeToZipFile(
   api.currentNote.id,
   'html', // or 'markdown'
   'trilium-export.zip')

Execute with ctrl-enter.

On my machine af ile is written to A:\apps\trilium-windows-x64\trilium-export.zip, which is my trilium application dir.

So if the zip filename were changed to, say, trilium-safe-mode.bat or resources.pak bad stuff happens. I've verified that doing so quietly overwrites existing files.

Not tested, but I presume creativity with other paths can overwrite files anywhere the trilium.exe executing user has write access to.

Mitigation

These are just ideas. I don't know what's best.

  • Before executing any script, ensure the current dir is user or system temp, at very least not application dir
  • disallow overwriting existing files
  • Alter exportSubtreeToZipFile to only accept fully qualifed path
    • repeat for any other functions that write to disk

Allowing arbitrary code execution is always going to be a security problem, that's not going away, it's too useful, but some relatively small changes could make it safer.

Error logs

No response

@perfectra1n
Copy link

Yeah this is a slippery slope, and in my opinion why it's important to run it in a container. Zadam had originally stated that Trilium was meant to hold a trusted user's notes - and that things like XSS and the sort weren't an issue if someone wanted to RCE themselves.

Now might be a good time to rehash that discussion though.

@maphew
Copy link
Author

maphew commented Oct 20, 2024

Running in a container doesn't prevent footgunning one's own install, which is what was in the neighbourhood of doing when I learned this. :) I wrote a script to export note tree, it ran successfully but I couldn't find the file, until I looked in the application dir. So some level of protection could be added, even before starting to look at things like cross domain exploits and remote code execution. I think we could/should do that too, but would be a different set of issues and discussion.

@perfectra1n
Copy link

perfectra1n commented Oct 20, 2024

If you run it in a container, and "footgun" yourself (causing the container to crash), the underlying container runtime should restart the container (unless you don't have restart policy set to always) causing the "footgunned" files to come back 👀

And you also can't overwrite binaries (to then be malicious, in theory) on the host OS if you're using a container.

But yes, if you overwrite any key files in a mounted volume (e.g. document.db, config.ini) - that wouldn't be a good time and is a different story.

@perfectra1n
Copy link

Yeah it'll definitely be interesting to try and sandbox the JavaScript provided by the user, changing how user's scripts execute.

@eliandoran eliandoran added the bug Something isn't working label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants