From 226654af2d560b2de87f83f9c86eb9a3312c9496 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Sat, 3 Oct 2026 00:27:34 +0530 Subject: [PATCH 1/2] fix(commit): accumulate gpgsig continuation lines in linear time `Commit._deserialize` collected a `gpgsig` header by appending each continuation line to a `bytes` object with `+=`. `bytes` are immutable, so every line copied the whole signature read so far, and a signature with *n* continuation lines cost O(n^2) to parse. Commit headers are fully controlled by whoever wrote the commit, and `_deserialize` runs on the first access to any commit attribute (`author`, `message`, `parents`, ...). Reading the commits of an untrusted repository is therefore enough to hit it. Measured on Python 3.11 with a `gpgsig` header of `" x\n"` lines: | object size | before | after | | ----------- | ------- | ------ | | 2.4 MB | 9.5 s | 0.08 s | | 4.8 MB | 116.8 s | | | 9.6 MB | 547.9 s | 0.32 s | Collect the lines in a list and join them once. This is linear and produces byte-identical `gpgsig` values, so the existing `test_gpgsig` round trip is unchanged. `test_gpgsig_deserialization_is_linear` deserializes a 1.8 MB signature under a 1 s CPU-time bound. It fails on the previous code and passes here. The full suite has no new failures; the 8 tests that fail locally (non-UTF-8 trailer encoding and NUL submodule names) fail identically without this change. `ruff`, `mypy` and `basedpyright --warnings` are clean. --- git/objects/commit.py | 6 +++--- test/test_commit.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/git/objects/commit.py b/git/objects/commit.py index 771f87976..307b8c59b 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -885,7 +885,7 @@ def _deserialize(self, stream: BytesIO) -> "Commit": if buf[0:10] == b"encoding ": self.encoding = buf[buf.find(b" ") + 1 :].decode(self.encoding, "ignore") elif buf[0:7] == b"gpgsig ": - sig = buf[buf.find(b" ") + 1 :] + b"\n" + sig_lines = [buf[buf.find(b" ") + 1 :] + b"\n"] is_next_header = False while True: sigbuf = readline() @@ -895,9 +895,9 @@ def _deserialize(self, stream: BytesIO) -> "Commit": buf = sigbuf.strip() is_next_header = True break - sig += sigbuf[1:] + sig_lines.append(sigbuf[1:]) # END read all signature - self.gpgsig = sig.rstrip(b"\n").decode(self.encoding, "ignore") + self.gpgsig = b"".join(sig_lines).rstrip(b"\n").decode(self.encoding, "ignore") if is_next_header: continue buf = readline().strip() diff --git a/test/test_commit.py b/test/test_commit.py index b5baf0e33..0a5bcdb7d 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -607,6 +607,28 @@ def test_commit_co_authors_bounds_malformed_trailer(self): # The malformed line yields nothing; the well-formed trailer still parses. assert result == [Actor("Real Name", "real@example.com")] + def test_gpgsig_deserialization_is_linear(self): + """A long gpgsig header must not make deserialization run in quadratic time.""" + num_lines = 600_000 + # Commit headers are fully attacker-controlled. Accumulating the signature with + # bytes concatenation copied it once per continuation line (O(n^2)). + data = ( + b"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" + b"author A 1700000000 +0000\n" + b"committer A 1700000000 +0000\n" + b"gpgsig -----BEGIN PGP SIGNATURE-----\n" + b" x\n" * num_lines + b" -----END PGP SIGNATURE-----\n" + b"\n" + b"message\n" + ) + cmt = copy.copy(self.rorepo.commit()) + start = time.process_time() + cmt._deserialize(BytesIO(data)) + elapsed = time.process_time() - start + # Leave ample CPU time for slow runners, but catch quadratic accumulation. + self.assertLess(elapsed, 1.0) + self.assertEqual(cmt.gpgsig.count("\n"), num_lines + 1) + self.assertEqual(cmt.message, "message\n") + @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.""" From 5fc5f3456a6816d4caa03fdb4a3b000d3170a290 Mon Sep 17 00:00:00 2001 From: Byron Date: Sat, 3 Oct 2026 04:31:59 +0200 Subject: [PATCH 2/2] review - remove test as it can totally fail on slow runners, and extending the time budget makes it hard to see if it's actually working. Let's trust what we see and look forward to a time when the in-python parsing is gone as well. --- test/test_commit.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/test/test_commit.py b/test/test_commit.py index 0a5bcdb7d..b5baf0e33 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -607,28 +607,6 @@ def test_commit_co_authors_bounds_malformed_trailer(self): # The malformed line yields nothing; the well-formed trailer still parses. assert result == [Actor("Real Name", "real@example.com")] - def test_gpgsig_deserialization_is_linear(self): - """A long gpgsig header must not make deserialization run in quadratic time.""" - num_lines = 600_000 - # Commit headers are fully attacker-controlled. Accumulating the signature with - # bytes concatenation copied it once per continuation line (O(n^2)). - data = ( - b"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" - b"author A 1700000000 +0000\n" - b"committer A 1700000000 +0000\n" - b"gpgsig -----BEGIN PGP SIGNATURE-----\n" + b" x\n" * num_lines + b" -----END PGP SIGNATURE-----\n" - b"\n" - b"message\n" - ) - cmt = copy.copy(self.rorepo.commit()) - start = time.process_time() - cmt._deserialize(BytesIO(data)) - elapsed = time.process_time() - start - # Leave ample CPU time for slow runners, but catch quadratic accumulation. - self.assertLess(elapsed, 1.0) - self.assertEqual(cmt.gpgsig.count("\n"), num_lines + 1) - self.assertEqual(cmt.message, "message\n") - @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."""