Autolinker vulnerable to RTLO URL spoofing attacks

[dbrgn]

When a URL string contains a unicode RTLO (right-to-left override) character, this results in two separate links being generated.

Context: https://www.bleepingcomputer.com/news/security/url-rendering-trick-enabled-whatsapp-signal-imessage-phishing/

PoC (screenshot, because GitHub would otherwise show the rest of the line as right-to-left):

Note that my terminal ignores the RTLO, but it's there. In the browser (Chrome in this case), this link looks like this:

The left and right part are separate links. When the user clicks on the right part, shown as aisa.mp4, they are redirected to http://4pm.asia/ instead.

I'm not sure what the ideal solution would be. If autolinker would strip all RTLO characters passed to it, then legitimate text (that's not part of an URL) might break. However, an RTLO should not be part of the actual URL. (Autolinker does basically no URL encoding whatsoever. Maybe that should be changed? An urlencoded RTLO would be handled (and ignored) by the browser.)

[caleb15]

@gregjacobs wondering if you saw this? Snyk lists this as a high-level vulnerability.

[ShaharAdskAcc]

Any idea when it's going to be fixed?

[Gilgahex]

@gregjacobs
@olafleur

Please see this pull request

#382

[moblezin]

Hey, thanks for the fix, when will you be able to publish it to npm?

[Gilgahex]

I'd love to but I don't have write access, so I'd have to fork it and create a new pkg on npm

[RomekRJM]

As mentioned by @caleb15 this is high on SNYKs vulnerability risk. Any idea when the next Autolinker release is going to be? I see past 2 years it's been released annually.

[gregjacobs]

Hey guys, waiting on a viable PR for this. The PR linked in this thread doesn't seem to do anything (although I'm surprised to see that the test passed. Perhaps there is a difference in which node version is running?)

[yadue]

@gregjacobs so you're saying that the fix provided doesn't work when you run it but units went correctly and fixed the original issue? I simply cannot understand that. Provided PR does exactly what it is supposed to do and the community is waiting for this PR for ages already.

Comment out
url.replace('\u202E', '');
and you'll see the result. Provided PR works fine, please merge it and deploy new version.

[xfournet]

@yadue when i test it, the PR is not working, the added test test fails with or without the patch in url-match.ts (see comments in PR)

[yadue]

@xfournet yeah indeed, yesterday it worked simply fine for me using this PR, today it's not which is extremely weird. I'll work on fix.

[gregjacobs]

Hey guys, this has been fixed and released in v3.16.1 👍