From 8bacd4da4fa6de578b818aa7a4b36bbeaaa243d7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 01 2021 10:34:40 +0000 Subject: Warn users when a PR contains some characters Unicode bi-directional characters can be present but unseen and thus missed during the review. With this PR, we create a list of characters that we want to warn the users about if present in a PR. Since that list is configurable, it can be extended as needed/desired. This relates to the CVE: CVE-2021-42574 and CVE-2021-42694 More can be found about it at: https://access.redhat.com/security/vulnerabilities/RHSB-2021-007 Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 44bec54..8f74ebc 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1931,6 +1931,20 @@ used. Defaults to: ``None`` (which results in the default branch being ``master``). +PR_WARN_CHARACTERS +~~~~~~~~~~~~~~~~~~ + +List of characters that triggers a warning to the users when met in a commit of +a pull-request (each commit being made checked). + +Defaults to: +:: + + set([ + chr(0x202a), chr(0x202b), chr(0x202c), chr(0x202d), chr(0x202e), + chr(0x2066), chr(0x2067), chr(0x2068), chr(0x2069) + ]) + RepoSpanner Options ------------------- diff --git a/pagure/default_config.py b/pagure/default_config.py index ac6c812..4aa1051 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -643,3 +643,17 @@ CSP_HEADERS = ( "base-uri 'self';" "img-src 'self' https:;" ) + +PR_WARN_CHARACTERS = set( + [ + chr(0x202A), + chr(0x202B), + chr(0x202C), + chr(0x202D), + chr(0x202E), + chr(0x2066), + chr(0x2067), + chr(0x2068), + chr(0x2069), + ] +) diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 76bc763..6047b78 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -6216,3 +6216,26 @@ def update_ticket_board_status( user=user, notification=True, ) + + +def find_warning_characters(repo_obj, commits): + """Return whether the given list of commits of the specified repository + object contains forbidden characters or not. + """ + warn_characters = pagure_config["PR_WARN_CHARACTERS"] + + for commit in commits: + if commit.parents: + diff = repo_obj.diff(commit.parents[0], commit) + else: + # First commit in the repo + diff = commit.tree.diff_to_tree(swap=True) + + for patch in diff: + for hunk in patch.hunks: + for line in hunk.lines: + subset = [c for c in line.content if c in warn_characters] + if subset: + return True + + return False diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html index 8e53c2d..9fcd1e5 100644 --- a/pagure/templates/repo_pull_request.html +++ b/pagure/templates/repo_pull_request.html @@ -132,7 +132,8 @@ -
+
+
{% if pull_request.status == 'Open' %} {% if g.authenticated and (g.fas_user.username == pull_request.user.username or g.repo_committer) %} @@ -161,7 +162,7 @@ {% endif %} - +
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index e4613d2..0ba221f 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -320,6 +320,10 @@ def request_pull(repo, requestid, username=None, namespace=None): if diff: diff.find_similar() + warning_characters = pagure.lib.query.find_warning_characters( + repo_obj, diff_commits + ) + form = pagure.forms.MergePRForm() trigger_ci_pr_form = pagure.forms.TriggerCIPRForm() @@ -369,6 +373,7 @@ def request_pull(repo, requestid, username=None, namespace=None): trigger_ci=trigger_ci, trigger_ci_pr_form=trigger_ci_pr_form, flag_statuses_labels=json.dumps(pagure_config["FLAG_STATUSES_LABELS"]), + warning_characters=warning_characters, )