| Message ID | 20170704031535.18151-1-eschwartz93@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 04/07/17 13:15, Eli Schwartz wrote: > As per https://lists.archlinux.org/pipermail/arch-general/2017-July/043876.html > git doesn't check that the tag name matches what an annotated tag object > *thinks* it should be called. This is a bit of a theoretical attack and > some would argue that we should always use commits since upstream can > legitimately change a tag, but nevertheless this can result in a > downgrade attack if the git download transport was manipulated. > > So, check the tag blob to make sure the tag actually matches the name we > used for `git checkout` > > Signed-off-by: Eli Schwartz <eschwartz93@gmail.com> This should be fixed in git.
On 07/03/2017 11:19 PM, Allan McRae wrote:
> This should be fixed in git.
Not that I disagree, but it appears this potential attack was discovered
a year ago, and sangy has in the meantime (since I started writing this
patch and before I emailed it and then noticed his link) posted a link
on IRC to the upstream discussion on fixing this in git(1).
http://public-inbox.org/git/20161007210721.20437-1-santiago@nyu.edu/
It would appear that upstream believes the fix is by adding git
porcelain/plumbing around the process of determining $tagname (rather
than relying on git-cat-file(1) piped to awk) and allowing git itself to
consider such tags as valid refs in the git repository.
Or, actually, I am not sure when this was added or what that patchset
accomplished, because it appeared to target `git tag -v` and `git
verify-tag`... by completely deleting everything but the now-valid
--format value, while presumably `git tag -l` which always accepted
--format continued to do what it always did?
What I do know is that the current version of git accepts
--format='%tag' everywhere relevant, and checking for this kind of
modification can and apparently should be done by every single project
or human interface that wants to make sure it isn't a problem. Go figure.
So I guess we could use instead:
tagname="$(git -C "$dir" tag -l --format='%(tag)' "$fragval")"
diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 5468f977..3783dbb2 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -187,6 +187,13 @@ verify_git_signature() { printf " %s git repo ... " "${dir##*/}" >&2 + tagname="$(git -C "$dir" cat-file tag "$fragval" 2>/dev/null | awk 'FNR == 3 {print $2}')" + if [[ $fragtype = tag && -n $tagname && $tagname != $fragval ]]; then + printf "%s (%s)" "$(gettext "FAILED")" "$(gettext "forged tag, you have been hacked!")" >&2 + errors=1 + return 1 + fi + git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1 if ! grep -qs NEWSIG "$statusfile"; then printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2
As per https://lists.archlinux.org/pipermail/arch-general/2017-July/043876.html git doesn't check that the tag name matches what an annotated tag object *thinks* it should be called. This is a bit of a theoretical attack and some would argue that we should always use commits since upstream can legitimately change a tag, but nevertheless this can result in a downgrade attack if the git download transport was manipulated. So, check the tag blob to make sure the tag actually matches the name we used for `git checkout` Signed-off-by: Eli Schwartz <eschwartz93@gmail.com> --- scripts/libmakepkg/integrity/verify_signature.sh.in | 7 +++++++ 1 file changed, 7 insertions(+)