Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when regenerating a patch with unquoted spaces in filename #6244

Merged
merged 3 commits into from
Apr 10, 2022

Conversation

jorio
Copy link
Contributor

@jorio jorio commented Mar 13, 2022

This PR fixes a null pointer deref in the following scenario:

  1. Write a patch that creates or deletes a file (with new file mode ... or delete file mode ...), and has unquoted filenames with spaces in the 1st line of the header.
    For example, take PATCH_ORIGINAL_NEW_FILE_WITH_SPACE in patch_common.h:
    diff --git a/sp ace.txt b/sp ace.txt
    new file mode 100644
    index 000000000..789819226
    --- /dev/null
    +++ b/sp ace.txt
    @@ -0,0 +1 @@
    +a
  2. Get a git_diff from the patch with git_diff_from_buffer
  3. Regenerate a patch with git_diff_to_buf

I'm not sure the proposed fix is optimal, since it doesn't reproduce the same diff --git header line. But, it at least prevents the crash, and vanilla git apply accepts the resulting patch.

This currently crashes, proposed fix in subsequent commit.
This fixes a crash in test cases
test_diff_parse__new_file_with_space_and_regenerate_patch
and
test_diff_parse__delete_file_with_space_and_regenerate_patch
@ethomson ethomson added the v1.4 label Apr 10, 2022
@ethomson
Copy link
Member

Thanks for the fix @jorio - I'm adding a minor formatting changes to match the libgit2 style a little bit better. I really appreciate the fix and the very nice tests.

src/libgit2/diff_print.c Outdated Show resolved Hide resolved
@ethomson ethomson merged commit 606afed into libgit2:main Apr 10, 2022
@ethomson ethomson mentioned this pull request Apr 12, 2022
@ethomson ethomson added the bug label Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants