Privatebin: i18n failed to handle datetime string properly

[Haocen]

Steps to reproduce

1. create a new paste.
2. click the email button and choose to send email in current timezone.

Additional info:

What happens

The timestamp was garbled:

The root caused seemed to be call to I18n._() unexpectedly modified the datetime string:

What should happen

The call to I18n._() should not modify the datetime string.

Additional information

Basic information

Server address:
privatebin.net

Server OS:
N/A

Webserver:
N/A

Browser:
Chrome Version 79.0.3945.117 (Official Build) (64-bit)

PrivateBin version:
1.3.2

I can reproduce this issue on https://privatebin.net: Yes

[rugk]

Indeed likely a bug introduced by the XSS fix in v1.3.2.

[elrido]

The commit introducing this is 4bf7f863dc2ffea1ea105e575205ab0f83ed2751.

Given we introduced this due to a security vulnerability, I'm hesitant to weaken this again. I'd prefer of someone else then me could take a look and suggest an improvement, commit or raise a PR to address this, without reintroducing the vulnerability.

---

The issue occurs in the following section of code:
https://github.com/PrivateBin/PrivateBin/blob/4bf7f863dc2ffea1ea105e575205ab0f83ed2751/js/privatebin.js#L621-L647

We now encode HTML entities in all parameters. We also encode the (translated) message ID, but only if it doesn't contain a link (<a/>-tag in HTML).

I guess the issue happens on messages that don't contain links, as they get inserted using jQuery's text(), which also applies HTML encoding, hence it currently double encodes these (quotes, in this case).

One may be tempted to just use the html() method in all cases, since now the encoding of entities is already handled. But given the history of this bug, I wouldn't want to repeat that misjudgement a second time. Hence I kindly ask you all to propose solutions for discussion and review by more folks then just @rugk and me.

[rugk]

I would propose to remove the "double encodes", i.e. as far as I see there are two code paths that are secure:

• containsNoLinks -> text (secure, because jQuery does all encoding/escaping or so)
• !containsNoLinks -> $element.html after DOMPurify.sanitize

Thus IMHO we can just remove these lines that do encode the HTML manually.

[elrido]

Correct, that's the two paths we may take, but we do still need those lines for the !containsNoLinks path, as neither $element.html() nor DOMPurify.sanitize() will do so, which was the bug we fixed in #554. So move them down into that path, including the sprintf() call?

I have to compensate this week for last weeks time efforts in preparing the release. If someone can put together a PR with the above change, I am happy to help review it over the next weekend.

I'm also considering improving the unit test by forcing it to go down both of those paths, as it still mostly tests the containsNoLinks one, as most generated strings contain no "

[rugk]

I am at a PR