fix(trailers): patch single-line bypass in shadow commit subjects - #2176
Open
Beehtrix wants to merge 2 commits into
Open
fix(trailers): patch single-line bypass in shadow commit subjects#2176Beehtrix wants to merge 2 commits into
Beehtrix wants to merge 2 commits into
Conversation
The new forgery tests cover newline-based injection, but they don't cover the remaining bypass where the subject is a single line that begins with a trailer key (e.g. "Entire-Session: attacker"). Adding a regression assertion for that case will ensure the fix actually prevents line-start trailer forgery.
flattenSubject() collapses whitespace, but a hostile single-line subject that starts with an Entire trailer key (e.g. "Entire-Session: attacker") will still be parsed as the first trailer because the regexes are anchored to line-start and the subject is the first line. This leaves a trailer-forgery bypass even without newlines.
Collaborator
|
These landed against feature branches rather than main - that's what the GitHub edit button does when you're browsing a branch. The branch owner has no way to find out: GitHub doesn't notify anyone when a PR is opened against their branch, and no cross-reference appears in the original PR's timeline either. For findings about unmerged work, a review comment on the original PR (with a suggested change) gets it to the person who can act on it and survives their next force-push. For findings about released code, open against main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Edge case: a single-line bypass.
Because the regexes are anchored to the line-start, a hostile single-line subject that begins exactly with a trailer key (e.g.,
Entire-Session: attacker) is still evaluated as line 1 and successfully parsed as a trailer.This layers onto your forgery fix by:
flattenSubject()to check if the flattened subject starts withEntire-. If it does, it prepends(subject)to safely neutralize the line-start anchor.trailer_forgery_test.goto explicitly pin this single-line bypass behavior.