-
Notifications
You must be signed in to change notification settings - Fork 533
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
Allow disabling 'Former-commit-id' footer & 'yyy [formerly xxx]' #8
Comments
Note that this doesn't mean we want to completely disable Object-Id substitution - in fact it's an argument for it, because we need a way to eliminate old commit id's from messages. Instead of 'yyy [formerly xxx]' we want 'yyy [formerly unclean commit]' or something similar. |
This was fixed by afabd06. |
I was just looking for an option to avoid the "Former-commit-id:" and hit this.. just in case anyone else is, FYI the new option added here is --private. (Oh and.. @rtyley thanks A LOT for BFG! It's AMAZING e.g. to clean up after a git svn clone massive import.) |
@vorburger so glad to hear that 😄 |
Need to think about the cryptographic security implications of these fields for the case where the user is filtering files to remove passwords/credentials.
If the former-object-id values are present, and an attacker wishes to establish what the '_REMOVED_' text originally was, they can run a brute force attack, creating a series of probable passwords and then calculating the tree hash for that tree, then the commit hash for that putative original commit (it may well be easy to determine the remainder of the original state for that commit object, chop off added footer, establish original parent commit id, etc). If that commit hash is identical, they have a strong contender for the original password.
Every hexadecimal digit of the former-commit-id we make available reduces the attacker's search space by a factor 16.
It's also perfectly true that all credentials should be changed anyway before publicising a repo, obviously there's a risk that a user might not do so, soit's best not to leave this vulnerability.
The text was updated successfully, but these errors were encountered: