Skip to content

Commit 007c562

Browse files
committed
Fix NULL dereference in refdb backend exists callback
When the Python exists callback raises, PyObject_CallObject returns NULL and pygit2_refdb_backend_exists jumped to its cleanup block, where Py_DECREF(result) dereferenced a NULL pointer, crashing the interpreter. It also returned 0 on the error path, swallowing the exception. Use Py_XDECREF and propagate the error instead. Add defensive Py_XDECREF to the has_log and ensure_log callbacks as well. Fixes #1476 Assisted-by: Kimi Code
1 parent 5878864 commit 007c562

3 files changed

Lines changed: 24 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
[#1471](https://github.com/libgit2/pygit2/issues/1471)
2424
[#1474](https://github.com/libgit2/pygit2/issues/1474)
2525
[#1475](https://github.com/libgit2/pygit2/issues/1475)
26+
[#1476](https://github.com/libgit2/pygit2/issues/1476)
2627

2728
- Update wheels to libgit2 1.9.6 and OpenSSL 3.5.7
2829

src/refdb_backend.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,8 @@ pygit2_refdb_backend_exists(int *exists,
237237
*exists = PyObject_IsTrue(result);
238238

239239
out:
240-
Py_DECREF(result);
241-
return 0;
240+
Py_XDECREF(result);
241+
return err;
242242
}
243243

244244
static int
@@ -423,6 +423,7 @@ pygit2_refdb_backend_has_log(git_refdb_backend *_be, const char *refname)
423423
Py_DECREF(args);
424424

425425
if ((err = git_error_for_exc()) != 0) {
426+
Py_XDECREF(result);
426427
return err;
427428
}
428429

@@ -449,6 +450,7 @@ pygit2_refdb_backend_ensure_log(git_refdb_backend *_be, const char *refname)
449450
Py_DECREF(args);
450451

451452
if ((err = git_error_for_exc()) != 0) {
453+
Py_XDECREF(result);
452454
return err;
453455
}
454456

test/test_refdb_backend.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,25 @@ def test_exists(repo: Repository) -> None:
139139
assert repo.backend.exists('refs/heads/master')
140140

141141

142+
class RaisingRefdbBackend(ProxyRefdbBackend):
143+
"""A backend whose exists callback always raises."""
144+
145+
def exists(self, ref: str) -> bool:
146+
raise RuntimeError('boom')
147+
148+
149+
def test_exists_callback_raises(testrepo: Repository) -> None:
150+
# Regression test (issue #1476): when the exists callback raises, the C
151+
# wrapper must not crash on a NULL result, and must propagate the error.
152+
backend = RaisingRefdbBackend(pygit2.RefdbFsBackend(testrepo))
153+
# Call the unbound C method so the call goes through the C wrapper
154+
# (pygit2_refdb_backend_exists), not directly to the Python override.
155+
# The error propagates as GitError, or as OSError/ValueError when a stale
156+
# libgit2 error (with a matching class) is left over from an earlier call.
157+
with pytest.raises((pygit2.GitError, OSError)):
158+
pygit2.RefdbBackend.exists(backend, 'refs/heads/master')
159+
160+
142161
def test_lookup(repo: Repository) -> None:
143162
assert repo.backend.lookup('refs/heads/does-not-exist') is None
144163
assert repo.backend.lookup('refs/heads/master').name == 'refs/heads/master'

0 commit comments

Comments
 (0)