request_password_reset: insufficiently escaped URL is incorrectly converted turned into a clickable link by GMail receiving plain text email

[mathieulb]

New Issue Checklist

• Report security issues confidentially.
• Any contribution is under this license.
• Before posting search existing issues.

Issue Description

If the username ends with punctuation (e.g. closing parenthesis), the password change URL is made clickable by GMail, except for that character, which causes a redirection to invalid_link.html.

Steps to reproduce

Create a username ending with a closing parenthesis. Call Parse.User.requestPasswordReset or the equivalent. Receive the plain text email. Email client detects a URL.

Actual Outcome

302 invalid_link.html

Expected Outcome

One of :
a) HTML message, so as to prevent link detection ;

b) a plain text message in which the link is somewhat more escaped than what is supposed to be required. Unfortunately I don't have a list of characters. The username is the only part of the link that has characters that might have to be escaped, so I say that you could escape all non \w characters in the username. E.g. user.username.replace(/\W/g, x=>"%"+x.charCodeAt(0).toString(16).padStart(2)) instead of encodeURIComponent(user.username) in sendPasswordResetEmail at lib/Controllers/UserController.js.

BTW there's another mail sender that doesn't get called, and whose buildEmailLink does not escape the username at all.

Environment

Server

• Parse Server version: 6.5.5
• Operating system: Heroku 22
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Heroku

Database

• System (MongoDB or Postgres): MongoDB
• Database version: 3.6.12
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): Heroku ObjectRocket

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): ParseObjC (thru Swift)
• SDK version: 2.7.0
• SDK (iOS, Android, JavaScript, PHP, Unity, etc): Android
• SDK version: 4.2.1
• SDK (iOS, Android, JavaScript, PHP, Unity, etc): JavaScript (browser)
• SDK version: 4.3.1

Logs

[parse-github-assistant]

Thanks for opening this issue!

• 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

[mtrezza]

Could you given an example of a username that would cause such an invalid link?

[mathieulb]

In the case that happened to us, a user added a smiley as part of her username, and that's not forbidden in our app, nor in Parse Server. I suppose that several other characters could cause the same problem, but I haven't tried them before. Now I have. Among all ASCII punctuations, they are ! " ' ) , . : ; < > ? ] ^ }

But the contents of that set are not totally obvious, such that another mail client might decide to exclude a different set of characters, so it's best to not take chances.

[mathieulb]

... the whole link looks like
https://myapp.herokuapp.com/parse/apps/0123456789ab-cdef-0123-456789abcdef/request_password_reset?token=3zgBwAwZ7mk5bAxyECGnVwK2h&username=Hello%20%3A)

and you can see that Github also excludes the closing parenthesis when turning that into a clickable link (when not using the explicit link feature of markdown with square brackets)

[mtrezza]

I assume there are restrictions as to what chars a username can contain, even if not documented. For example how about the username "🙂"?

Anyway, for this issue, adding the missing escape should be sufficient, I guess. But why is it not escaping the whole username in the first place?

[mathieulb]

Parse Server accepts the creation of a user named "🙂", as well as a user named "hello :)". Apps don't necessarily add restrictions of their own. Anyway, there is no reason to add any such restriction now, and it can't reasonably apply to users that have already been created anyway, and the only place where there is a problem with special chars is in request_password_reset.

The reason it's not escaping the whole username is because sendPasswordResetEmail uses encodeURIComponent, which doesn't do it because it's not required according to the spec of URLs. But GMail/GitHub/Facebook (etc) require escaping because people often put a closing parenthesis or square bracket or comma or period (etc) right after a URL in a sentence. In almost all cases, the URL didn't end with that punctuation, the punctuation was added by the user after pasting the URL. So the problem is that encodeURIComponent strictly follows the standard instead of dealing with certain non-conforming aspects of reality (that have good reasons to be non-conforming, imho).

But is there anything I have to do so that this change be applied ?

[mtrezza]

Well, if you would like to open a PR for this change?