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

Allow all URLs schemes for in-chat messages #18468

Closed
AgentOak opened this issue Aug 10, 2021 · 3 comments
Closed

Allow all URLs schemes for in-chat messages #18468

AgentOak opened this issue Aug 10, 2021 · 3 comments
Labels
A-URL-Previews O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@AgentOak
Copy link

AgentOak commented Aug 10, 2021

Steps to reproduce

  1. Open Element, enter any chat
  2. Write and send a message that contains a URL with a scheme other than http or https

What happened?

It looks like Element has a hard-coded list of schemes it will consider for a URL, therefore not converting some valid URLs into clickable links.

element-urls

The only link that is actually clickable in this image is the first one. file:// and ftp:// are recognized but don't do anything when clicked. All the other links where the scheme was cut off also don't do anything when you click on them.

What did you expect?

Element recognizes all URLs independently of their scheme and upon clicking them invokes the appropriate system handler to launch the default application registered for the scheme.

On the other hand, file:// URLs should probably be rejected, even though they work properly from the OS side, i.e. Win+R "file://C:/example.log" correctly opens the file in the default application for .log files.
I don't know about data: URIs but those probably cannot be handled properly so they're fine as is.

Operating system

Windows 10

Application version

Element version: 1.7.34, olm version: 3.2.3

How did you install the app?

https://element.io/get-started

@aaronraimist
Copy link
Collaborator

Related/dup: #4457 #8720 #15018

@AgentOak
Copy link
Author

AgentOak commented Aug 10, 2021

So there is an issue with linkifyjs not recognizing valid URLs (and seemingly being unmaintained), and additionally there is a filter list of schemes in matrix that applies even to markdown-formatted URLs?

https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/HtmlUtils.tsx#L60
What a strange idea to arbitrarily filter schemes instead of leaving it to the operating system like browsers do.
I had seen #4457 but individually adding schemes to said list is a never-ending story given the hundreds of different schemes in use out there.

Re matrix-org/matrix-react-sdk#6388:

It seems like Element should probably just allow all the same schemes that Firefox allows https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler.

That is NOT the list of schemes Firefox allows for links. It is the list of schemes that Firefox allows web apps to register a handler for. It is irrelevant to <a> links. For example, steam isn't on the list but Firefox handles it as expected:

firefox-steam-scheme

@germain-gg germain-gg changed the title Element parses URLs in chat messages incorrectly Allow all URLs schemes for in-chat messages Aug 10, 2021
@germain-gg germain-gg added A-URL-Previews O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist labels Aug 10, 2021
@germain-gg
Copy link
Contributor

Thank you for filing this issue

Element Web is filtering some URL scheme to prevent opening up new attack vector. It is unlikely that we will ever open all URL scheme but we will rather accept some new scheme on a case by case basis

Some schemes like javascript: are unsafe, and things like CSP can mitigate it, but it is probably not the safest bet to only rely on that to prevent this type of attacks.
This article summarises a lot of problems that can occur when allowing any arbitrary URLs https://positive.security/blog/url-open-rce

For the reasons mentioned above, I will close this ticket. And as I mentioned we will welcome pull request and review/allow them on a case by case basis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-URL-Previews O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

3 participants