We can pass a few const char * to git_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.
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.