Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

plumbing/object: fix pgp signature encoder/decoder #892

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

jfontan
Copy link
Contributor

@jfontan jfontan commented Jul 18, 2018

The way of reading pgp signatures was searching for pgp begin line in the header. This caused problems when this string appeared and was not part of the signature. For example if it appears in the message as an example or is part of the author name the decoder starts treating it as a signature. In this state the code was not able to notice then the header ended so it entered in an infinite loop searching for pgp end string.

Now it uses the same method as original git. Searches for gpgsig section in header and starts getting all lines until the next part.

In encoder the string used to add signatures was incorrect. It is now changed to the proper gpgsig string instead of pgpsig.

The way of reading pgp signatures was searching for pgp begin line in
the header. This caused problems when this string appeared and was not
part of the signature. For example if it appears in the message as an
example or is part of the author name the decoder starts treating it as
a signature. In this state the code was not able to notice then the
header ended so it entered in an infinite loop searching for pgp end
string.

Now it uses the same method as original git. Searches for gpgsig section
in header and starts getting all lines until the next part.

In encoder the string used to add signatures was incorrect. It is now
changed to the proper "gpgsig" string instead of "pgpsig".

Signed-off-by: Javi Fontan <[email protected]>
Copy link
Collaborator

@smola smola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the fix also prevent the function to hang up if there is a malformed / malicious commit with crafted gpgsign header?

@ajnavarro
Copy link
Contributor

ping @jfontan

@jfontan
Copy link
Contributor Author

jfontan commented Jul 24, 2018

@smola I believe it does. Now gpgsign header read is stopped when the line is empty or its first character is not space. https://github.com/src-d/go-git/pull/892/files#diff-7eac18321111ba95fe06592bca6b5112R185

This fixes the infinite loop past EOF we had before. I can not think on other way to exploit signatures.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants