From 4eb5a22a111a05794774f0437864ace387aa4689 Mon Sep 17 00:00:00 2001
From: AliAkbar <aliakbarit019@gmail.com>
Date: Mon, 25 Sep 2023 00:06:53 +0500
Subject: [PATCH] fix: add XSS checks to validation for abbr attributes

---
 i18n/validate.py                    | 10 ++++++++--
 requirements/base.in                |  1 +
 requirements/base.txt               |  3 +++
 requirements/common_constraints.txt |  5 -----
 tests/data/validation_problems.po   |  4 ++++
 tests/test_validate.py              |  6 ++++++
 6 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/i18n/validate.py b/i18n/validate.py
index 3d84ef55..aa33d788 100644
--- a/i18n/validate.py
+++ b/i18n/validate.py
@@ -9,11 +9,12 @@
 import textwrap
 
 import polib
+from lxml.html import clean
 
+from i18n import Runner
+from i18n.converter import Converter
 from i18n.dummy import is_format_message
 from i18n.execute import call
-from i18n.converter import Converter
-from i18n import Runner
 
 log = logging.getLogger(__name__)
 
@@ -97,6 +98,11 @@ def is_linguistic_tag(tag):
         if tag.startswith("&"):
             return True
         if any(x in tag for x in ["<abbr>", "<abbr ", "</abbr>"]):
+            if "<abbr " in tag:
+                cleaned_tag = clean.clean_html(tag)
+                # clean_html will remove XSS from tag so check so don't skip abbr tag if cleaned_tag is different
+                if cleaned_tag != tag:
+                    return False
             return True
         return False
 
diff --git a/requirements/base.in b/requirements/base.in
index 317290d2..a5285ef9 100644
--- a/requirements/base.in
+++ b/requirements/base.in
@@ -5,3 +5,4 @@ Django
 polib
 path
 pyYaml
+lxml
diff --git a/requirements/base.txt b/requirements/base.txt
index 1e71932c..3944a101 100644
--- a/requirements/base.txt
+++ b/requirements/base.txt
@@ -10,6 +10,9 @@ django==3.2.17
     # via
     #   -c requirements/common_constraints.txt
     #   -r requirements/base.in
+lxml==4.9.3
+    # via
+    #   -r requirements/base.in
 path==16.6.0
     # via -r requirements/base.in
 polib==1.1.1
diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt
index a9878ac2..d34ab34d 100644
--- a/requirements/common_constraints.txt
+++ b/requirements/common_constraints.txt
@@ -30,8 +30,3 @@ django-simple-history==3.0.0
 # tox>4.0.0 isn't yet compatible with many tox plugins, causing CI failures in almost all repos.
 # Details can be found in this discussion: https://github.com/tox-dev/tox/discussions/1810
 tox<4.0.0
-
-# edx-sphinx-theme is not compatible with latest Sphinx==6.0.0 version 
-# Pinning Sphinx version unless the compatibility issue gets resolved
-# For details, see issue https://github.com/openedx/edx-sphinx-theme/issues/197
-sphinx<6.0.0
diff --git a/tests/data/validation_problems.po b/tests/data/validation_problems.po
index ebc36b1c..890a6969 100644
--- a/tests/data/validation_problems.po
+++ b/tests/data/validation_problems.po
@@ -79,3 +79,7 @@ msgstr "Look -- a dog!"
 # <abbr> could come-and-go with translations
 msgid "The <abbr>CIA</abbr> said so"
 msgstr "The secret agency said so"
+
+# <abbr> may contain cross-site script attack which is usually skipped from validation
+msgid "No tags"
+msgstr "Added XSS tag <abbr title='Cascading Style Sheets' onmouseover='XSS ATTACK!!!'>CSS</abbr>"
diff --git a/tests/test_validate.py b/tests/test_validate.py
index f14c087a..92d3dace 100644
--- a/tests/test_validate.py
+++ b/tests/test_validate.py
@@ -59,6 +59,12 @@
         '"{nomx}" added',
     ),
     ('Empty translation', 'This string should not be empty'),
+    (
+        'Different tags in source and translation',
+        'No tags',
+        "Added XSS tag <abbr title='Cascading Style Sheets' onmouseover='XSS ATTACK!!!'>CSS</abbr>",
+        '"<abbr title=\'Cascading Style Sheets\' onmouseover=\'XSS ATTACK!!!\'>" added'
+    ),
 ]