[#35398] - [4][Security] Translation strings should be treated as UNSAFE
- Closed
- 1 Sep 2021
- Medium
- Build: 4.0-dev
- # 35398
- dgrammatiko:4.0—sanitize—Joomla.Text-dev
Pull Request for Issue # .
Summary of Changes
- Pass all the translation strings through the sanitizer
Why?
The translation strings often have HTML markup and many times are used in combination with innerHTML. BAD combo!!!
Also, there is a load function that could load any arbitrary JSON into the translation string pool (an object basically). Another backdoor for XSS.
Using the existing sanitizer these backdoors are closed.
Testing Instructions
Apply the PR
Run npm ci
Check that the strings everywhere in the backend/frontend are not affected
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Documentation Changes Required
Yes, there should be a note that inline JS events or any javascript: are not allowed anymore and will be discarded.
FWIW I already did a basic search on the languages folders and all strings are not affected (eg no funky inline events, etc)
Please reopen. This protects against naught translations as well.
too many files need adjustment
No one said it will be easy :)
For my opinion there is 80% cases where should be used textContent instead of innerHTML, and 10% should be corrected to use textContent and 10% where actually HTML is need to be filtered/sanitized.
Current approach gives overhead for runtime. Maybe it not that big, but still.
It is what I think.
Will wait what other will say.
Quy
- | Labels |
Added:
?
|
||
Current approach gives overhead for runtime. Maybe it not that big, but still.
I will have to agree with @Fedik here about the overhead and also it's a B/C break of the API if Joomla.Text._('WHATEVER') doesn't return the same string as it did before (it might happen if the sanitiser catches something it doesn't like/understands)
Hopefully someone else will lead the way and start patching the script files...
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-09-01 10:12:49 |
| Closed_By | ⇒ | dgrammatiko |
joomla-cms-bot
- | Category | ⇒ | JavaScript Repository NPM Change |
Technically you're correct, that should be the right thing to do. The catches here are:
Joomla.Textwill need to do that themselves and most of them won't...I'm ok closing this if the project decides to do this on a per-call of
Joomla.Text