diff --git a/git/objects/commit.py b/git/objects/commit.py index 5b73a3432..771f87976 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -10,7 +10,6 @@ from io import BytesIO import logging import os -import re from subprocess import Popen, PIPE from time import altzone, daylight, localtime, time, timezone import warnings @@ -964,12 +963,21 @@ def co_authors(self) -> List[Actor]: co_authors = [] if self.message: - results = re.findall( - r"^Co-authored-by: (.*) <(.*?)>$", - str(self.message), - re.MULTILINE, - ) - for author in results: - co_authors.append(Actor(*author)) + # Scan line by line instead of matching `(.*) <(.*?)>` across the whole + # message. On a single trailer line that repeats " <" without ever closing + # a ">", greedy backtracking over each " <" made the regex run in O(n^2) + # time, so a large (fully attacker-controlled) commit message could stall + # any caller of this property. A trailer is "Co-authored-by: " + # with the email in the final angle brackets, so the name ends at the last + # " <" and the line ends at ">". + prefix = "Co-authored-by: " + for line in str(self.message).split("\n"): + if not line.startswith(prefix) or not line.endswith(">"): + continue + identity = line[len(prefix) :] + separator = identity.rfind(" <") + if separator == -1: + continue + co_authors.append(Actor(identity[:separator], identity[separator + 2 : -1])) return co_authors diff --git a/test/test_commit.py b/test/test_commit.py index 431269b29..b5baf0e33 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -590,6 +590,23 @@ def test_commit_co_authors(self): Actor("test_user_3", "test_user_3@github.com"), ] + def test_commit_co_authors_bounds_malformed_trailer(self): + """A malformed trailer line must not make co_authors run in quadratic time.""" + commit = copy.copy(self.rorepo.commit("4251bd5")) + # An unterminated trailer repeating " <" without a closing ">". The old + # `(.*) <(.*?)>` regex backtracked over every " <" (O(n^2)); the crafted line + # is fully attacker-controlled through the commit message. + commit.message = ( + "Subject\n\nCo-authored-by: " + ("a <" * 20_000) + "\nCo-authored-by: Real Name " + ) + start = time.process_time() + result = commit.co_authors + elapsed = time.process_time() - start + # Leave ample CPU time for slow runners, but catch quadratic backtracking. + self.assertLess(elapsed, 1.0) + # The malformed line yields nothing; the well-formed trailer still parses. + assert result == [Actor("Real Name", "real@example.com")] + @with_rw_directory def test_create_from_tree_with_trailers_dict(self, rw_dir): """Test that create_from_tree supports adding trailers via a dict."""