Repository navigation
git_patch_from_blob_and_buffer has underspecified input lifetime #7318
Description
Activity
Would you want this one done? — Pin down and document whether
git_patch_from_blob_and_bufferneeds its input buffer to stay alive after the call returns, since two different Rust/Ruby bindings currently make opposite assumptions. If yes, we'll do it and open a PR; if not, say so (or just ignore this) and we won't touch it.How it works: an AI agent does the work against your write-up; it's independently checked in a clean container (your test suite, network off) before anything reaches you; a human reviews it, submits it, and answers the review. You keep full control — nothing lands unless you merge it.
Recent examples of the same flow on other projects: stretchr/testify#1932, pypa/pip-audit#1089, sigstore/sigstore-python#1845.
We're testing whether work offered this way is useful to maintainers — a blunt "no" is as helpful as a yes.
We can pass a few
const char *togit_patch_from_blob_and_buffer, particularly the buffer that it's going to diff to. The documentation does not say what happens to those strings. Dooes the library make a copy? Do we need to keep these strings allocated until we are done with the output objects?The default across libgit2 is that you do not need to extend the lifetime of inputs, and this is what at least rugged does, which can lead to problems if the garbage collector frees the input string before we're done printing the patch.
What is the correct lifetime for this function? In rugged it's pretty easy to keep the buffer alive by adding as an owner of the patch, but I see that git2-rs also considers that it does not need to keep the input alive, based on what I can see in https://github.com/rust-lang/git2-rs/blob/a34d746d4348ecac7318f0ba5508279505f9e9b6/src/patch.rs#L76-L98 and there you do actually have to specify lifetimes explicitly (there are ways with lifetime annotations but those would be super annoying here).
This was originally reported as a security issue in rugged by Yuhang Wu from depthfirst but I don't see that an attacker would have any influence on this and it seems like a regular programming error to me.