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

Desktop: Disallow UNC file links #9979

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Feb 22, 2024

Disallows Windows UNC file:// links.

@personalizedrefrigerator personalizedrefrigerator changed the title Desktop: Security: Disallow UNC file links Desktop: Disallow UNC file links Feb 22, 2024
@laurent22 laurent22 merged commit 836e23c into laurent22:dev Feb 22, 2024
9 of 10 checks passed
@Telefaust
Copy link

Any option to switch that off, or embed other way to click-to-open a Windows registered file type?
Looks like a major show stopper for me.
Thanks.

@personalizedrefrigerator
Copy link
Collaborator Author

Hi!

What's your use case for this? (Possible solutions depend what network file paths are being used for).

Note that this change only affects UNC file paths, not paths to local files.

@Telefaust
Copy link

Hi!
One of my tasks for Joplin is to build process flows that include steps as "get the data from that excel table" or "create a word doc using that template", and most of files are on windows network storages. Mapping storage volumes to drive letters is not a good case as I'm working from different places and my local network is not available everywhere. Therefore I'm using UNC links, the "file:///..." ones.

@PackElend
Copy link
Collaborator

build process flows
Out of curiosity, what do you use for that

Windows itself can be quite restrictive in the this regard

@Telefaust
Copy link

Either I've never met such "restrictions" from Windows or I've got used to them for years. If I click a file link - file opens, be it local or network one. If not, there is something wrong with the file or network. I can do it from desktop, from explorer window, from an email, from a document. In "most complicated" :) cases I have to ctrl-click, or use a right button menu. And yes, it worked within Joplin client too - till this update, of course, which I didn't install for that reason.

In other words, it was completely OK for me as it worked (removing one extra \ in md editor is not a problem).

Talking "processes" I'm talking not about Gantt charts or something like that. Not timing, but interworking. Instruction for user A says he must do this and this and on step X get information K from user B, therefore instruction for user B must include aggregating this information for sources 1 and 2 and providing to user A by email... that kind of thing. I've tried several "portable wikis", but note-app fits me better.

@PackElend
Copy link
Collaborator

I agree on the listed apps there is one major app you may overlooked TEAMS (and the Office online).

And, I guess, it will become increasingly a safety concern. So if Joplin allows this it should be controllable by Windows Policies as Joplin is used in professional environments as well.

@personalizedrefrigerator
Copy link
Collaborator Author

Mapping storage volumes to drive letters is not a good case as I'm working from different places and my local network is not available everywhere. Therefore I'm using UNC links, the "file:///..." ones.

I'm looking into making this re-enableable with a plugin (perhaps just for a specific notebook). Do you need UNC links to work in HTML notes (or just Markdown ones)?

@Telefaust
Copy link

I'm looking into making this re-enableable with a plugin (perhaps just for a specific notebook). Do you need UNC links to work in HTML notes (or just Markdown ones)?

Thank you.
I'm using UNC links in markdown notes only.

@laurent22
Copy link
Owner

I'm looking into making this re-enableable with a plugin (perhaps just for a specific notebook). Do you need UNC links to work in HTML notes (or just Markdown ones)?

Please wait before developing a plugin or working on this issue, let's first discuss our options. I think one possible fix is to revert this change - the security vulnerability that was reported was never about UNC paths and I don't know why it became about this

@Telefaust
Copy link

I think one possible fix is to revert this change
Sounds good for me too.

@personalizedrefrigerator
Copy link
Collaborator Author

I think one possible fix is to revert this change
Sounds good for me too.

See

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 this pull request may close these issues.

None yet

4 participants