Skip to content

fix(git): preserve trailing CR when splitting raw commit/tag objects - #871

Open
kanywst wants to merge 1 commit into
sigstore:mainfrom
kanywst:fix/f001-preserve-cr-in-rawobj-split
Open

fix(git): preserve trailing CR when splitting raw commit/tag objects#871
kanywst wants to merge 1 commit into
sigstore:mainfrom
kanywst:fix/f001-preserve-cr-in-rawobj-split

Conversation

@kanywst

@kanywst kanywst commented Aug 19, 2026

Copy link
Copy Markdown

Summary

SplitCommit, SplitTag and checkUniqueHeaders (pkg/git/rawobj.go) read lines with bufio.Scanner's default ScanLines, which drops a trailing \r. git-core doesn't, so what gitsign splits isn't the bytes git hashed.

A commit and its trailing-\r twin are separate objects that split to the same Payload, so one signature verifies for both. And a line that is just \r reads as blank at rawobj.go:459 and ends the header scan, so ValidateCommit misses a duplicate tree placed after it:

$ printf 'tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\nauthor A <a@example.com> 1 +0000\ncommitter C <c@example.com> 1 +0000\n\r\ntree 0000000000000000000000000000000000000001\n\nsubject\n' | git hash-object -t commit -w --literally --stdin
9e480e31e5b24674efa75b64861330eb7f9d9648
$ git log --format=%s -1 9e480e3
subject

git and go-git both keep reading headers past that line, so the duplicate is in the header section for everyone except the validator. Without the \r, the same object is rejected.

Fix splits on \n and keeps the \r. The four new test cases fail before it.

No exploit that I can find: a trailing \r is the only edit that leaves the payload unchanged, and the validator feeds the attest path only. Happy to take this private instead if you'd rather.

SplitCommit, SplitTag and checkUniqueHeaders read lines with
bufio.ScanLines, which drops a trailing '\r'. git doesn't, so what we split
isn't the bytes git hashed.

A commit and its trailing-'\r' twin are separate objects that split to the
same payload, so one signature verifies for both. And a line that is just
'\r' reads as blank and ends the header scan, so ValidateCommit and
ValidateTag miss a duplicate tree or object header placed after it.

Split on '\n' and keep the '\r'.

Signed-off-by: kanywst <niwatakuma@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant