Rocket.chat: Pipe character breaks emojis

[SarCaustic]

Description:

Vertical pipe character "|" breaks custom emoji loading. Emojis loaded before will work fine but all afterwards will not work.

Server Setup Information:

• Version of Rocket.Chat Server: 0.58.2
• Operating System: Ubuntu 16.04
• Deployment Method(snap/docker/tar/etc): snap
• Number of Running Instances: 1
• DB Replicaset Oplog:
• Node Version:

Steps to Reproduce:

1. create any emoji where the name or alias contains an "|"

Expected behavior:

Normal display and function of custom emoji

Actual behavior:

emojis do not load, there are listed in the search but no image is displayed and attempts to use it just sends the plain text.

Relevant logs:

server and browser both have no errors in logs.

[matheusml]

Do you have an example of an emoji that contains a "|", or ir this only happening with custom emojis?

[SarCaustic]

This only happens with custom emoji. There aren't any default emoji/smileys that have the pipe.

[matheusml]

After taking a deep dive, my recommendation for this is to simply disabled the use of pipe character in custom emojis.

We're using RegExp to parse the custom emojis, and as you can see on the line below, the pipe character is heavily used, which is causing the whole trouble:

/* emojiCustom.js */

const regShortNames = new RegExp(`<object[^>]*>.*?<\/object>|<span[^>]*>.*?<\/span>|<(?:object|embed|svg|img|div|span|p|a)[^>]*>|(${ RocketChat.emoji.packages.emojiCustom.list.join('|') })`, 'gi');

Not only that, but the alias name is used to get the asset of the image, with is going to break because browsers don't support URL withs pipes.

By the way, similar characters are already disabled, as you can see here:

/* insertOrUpdateEmoji.js */

//allow all characters except colon, whitespace, comma, >, <, &, ", ', /, \, (, )
//more practical than allowing specific sets of characters; also allows foreign languages
const nameValidation = /[\s,:><&"'\/\\\(\)]/;
const aliasValidation = /[:><&"'\/\\\(\)]/;

So, in my opinion we should close this, and open an issue to also disable the pipe character from custom emojis.

What you think @SarCaustic, @gdelavald ?

[SarCaustic]

Most of the recent browsers can encode the "|" character to %7C so that in itself isn't a problem as long as someone doesn't use an old version.
But since the pipe is a big part of RegEx I'd agree that that's probably the best option unless there was an easy alternative. For something like Emoji people can use a lowercase "L" or a capital "i" since it's all cosmetic anyways.

[matheusml]

@SarCaustic exactly! And we're already banning characteres like: comma, >, <, &, /, \, the pipe isn't that far away from them.

I'm just waiting for a definition from @gdelavald so that I can close this and open another one.

[gdelavald]

I agree with disabling the pipe character, unless @rodrigok has a different opinion, go ahead with the fix. No need to open another issue though, you can use this one to track it and just create a PR.

[rodrigok]

@matheusml did you try to escape the regex? Like:

const regShortNames = new RegExp(`<object[^>]*>.*?<\/object>|<span[^>]*>.*?<\/span>|<(?:object|embed|svg|img|div|span|p|a)[^>]*>|(${ s.escapeRegExp(RocketChat.emoji.packages.emojiCustom.list.join('|')) })`, 'gi');

[matheusml]

@rodrigok I tried escaping the pipe, not exactly like in your example, but I did.

But I don't think that's point. If we're already banning characters like comma, > and <, that are 'tricky' to RegExp, why not keep the pattern and also ban the pipe character?

It seems to me to be the safest decision.