Skip to content

fix(tag): split tag object headers at line feeds only - #2290

Open
Keerthana-64 wants to merge 1 commit into
gitpython-developers:mainfrom
Keerthana-64:tag-header-line-feeds
Open

Keerthana-64 wants to merge 1 commit into
gitpython-developers:mainfrom
Keerthana-64:tag-header-line-feeds

Conversation

@Keerthana-64

Copy link
Copy Markdown
Contributor

TagObject._set_cache_ split the decoded tag object with str.splitlines() and then read the headers by position, taking lines[3] as the tagger. str.splitlines() also breaks at \r, \v, \f, \x1c-\x1e, \x85, U+2028 and U+2029, but git ends a header only at a line feed. A U+2028 inside the tag header of an untrusted tag object therefore shifts every later header by one line in GitPython only. With tag v1<U+2028>tagger Linus Torvalds <torvalds@linux-foundation.org> 1700000000 +0000 followed by a real tagger Mallory <m@evil.example> ... line, git cat-file tag, git show and git for-each-ref --format=%(taggername) report Mallory, while GitPython reported tag == "v1" and Linus Torvalds as the tagger. The object passes git fsck --strict and a clone with transfer.fsckObjects=true, because git only warns badTagName.

The fix splits at "\n" and drops the trailing empty element, so tags that only use line feeds parse exactly as before. A \r or U+2028 inside the message is now kept, as git shows it. The new test in test/test_refs.py fails before the change and passes after it. ruff, ruff format --check and mypy are clean. On my machine the full suite has 3 failures (test_empty_repo, test_rebasing, test_diff_with_staged_file), and all 3 also fail without this change.

`TagObject._set_cache_` split the decoded tag object with `str.splitlines()` and then read the headers by position, taking `lines[3]` as the tagger. `str.splitlines()` also breaks at `\r`, `\v`, `\f`, `\x1c`-`\x1e`, `\x85`, U+2028 and U+2029, but git ends a header only at a line feed. A U+2028 inside the `tag` header of an untrusted tag object therefore shifts every later header by one line in GitPython only. With `tag v1<U+2028>tagger Linus Torvalds <torvalds@linux-foundation.org> 1700000000 +0000` followed by a real `tagger Mallory <m@evil.example> ...` line, `git cat-file tag`, `git show` and `git for-each-ref --format=%(taggername)` report Mallory, while GitPython reported `tag == "v1"` and Linus Torvalds as the tagger. The object passes `git fsck --strict` and a clone with `transfer.fsckObjects=true`, because git only warns `badTagName`.

The fix splits at `"\n"` and drops the trailing empty element, so tags that only use line feeds parse exactly as before. A `\r` or U+2028 inside the message is now kept, as git shows it. The new test in `test/test_refs.py` fails before the change and passes after it. `ruff`, `ruff format --check` and `mypy` are clean. On my machine the full suite has 3 failures (`test_empty_repo`, `test_rebasing`, `test_diff_with_staged_file`), and all 3 also fail without this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant