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

Commit

Permalink
Validate received URLs before cloning
Browse files Browse the repository at this point in the history
A malicious GitHub server could send an URL with the form
`ext::<command>` and that would run arbitrary code where the git-hub
command is ran. To avoid surprises, a simple heuristic is used to spot
fishy URLs (including any `<transport>::` URL or URLs that don't match
the urltype requested).

This should fix most of sociomantic-tsunami#197.
  • Loading branch information
leandro-lucarella-sociomantic committed Oct 6, 2016
1 parent 7f0f9d8 commit 3fb02d1
Showing 1 changed file with 39 additions and 6 deletions.
45 changes: 39 additions & 6 deletions git-hub
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,33 @@ def split_titled_message(msg):
body = '\n'.join(lines[2:])
return (title, body)

# Perform some basic checks on the URL to avoid MitM attacks
def validate_url(url, urltype):
scheme = urltype[:-4] # Remove the '_url'
# Any URL starting with <transport>:: will run
# a git-remote-<transport> command. "ext::" is specially
# unsafe, as that can run arbitrary commands.
if '::' in url:
pass # Fishy already, so skip to the question
elif scheme in ('clone', 'svn') and re.match(r'^https?://', url):
return
elif scheme == 'git' and re.match(r'^git://', url):
return
elif scheme == 'ssh' and (re.match(r'^ssh://', url) or
re.match(r'^[^@:]+@[^:]+:', url)):
return
else:
warnf("Unknown url type {}!", urltype)
answer = ask("The URL reported by the GitHub server ({!r}) looks quite "
"fishy! You might be under a man-in-the-middle attack "
"(https://en.wikipedia.org/wiki/Man-in-the-middle_attack). Do "
"you really trust your GitHub server ({})?"
.format(url, config.baseurl),
default='NO! ABORT!',
options=["NO! ABORT!", "yes, it's fine"])
if answer != "yes, it's fine":
die("Aborted because of potential MitM attack")


# Command-line commands helper classes
########################################################################
Expand Down Expand Up @@ -776,6 +803,7 @@ class CloneCmd (object):
parser.error("Can't use triangular workflow without "
"an upstream repo")
url = repo['parent'][urltype] if triangular else repo[urltype]
validate_url(url, urltype)
infof('Cloning {} to {}', url, dest)
git_quiet(1, 'clone', *(args.unknown_args + [url, dest]))
if not upstream:
Expand All @@ -791,6 +819,7 @@ class CloneCmd (object):
git_config('forkremote', value=remote)
git_config('urltype', value=urltype)
git_config('upstream', value=upstream)
validate_url(remote_url, urltype)
git('remote', 'add', remote, remote_url)
infof('Fetching from {} ({})', remote, remote_url)
git_quiet(1, 'fetch', remote)
Expand Down Expand Up @@ -1299,6 +1328,13 @@ class PullUtil (IssueUtil):
gh_head = config.forkrepo.split('/')[0] + ':' + remote_head
return head_ref, head_name, remote_head, base, gh_head

# Perform a validated git fetch
@classmethod
def git_fetch(cls, url, ref, urltype=config.urltype):
validate_url(url, urltype)
infof('Fetching {} from {}', ref, url)
git_quiet(1, 'fetch', url, ref)

# `git hub pull` command implementation
class PullCmd (IssueCmd):

Expand Down Expand Up @@ -1463,9 +1499,7 @@ class PullCmd (IssueCmd):
remote_url = pull['head']['repo'][config.urltype]
remote_branch = pull['head']['ref']

infof('Fetching {} from {}', remote_branch, remote_url)

git_quiet(1, 'fetch', remote_url, remote_branch)
cls.git_fetch(remote_url, remote_branch)
git_quiet(1, 'checkout', 'FETCH_HEAD', *args.args)

# This class is top-level just for convenience, because is too big. Is added to
Expand Down Expand Up @@ -1782,15 +1816,14 @@ class RebaseCmd (PullUtil):
old_ref = old_ref_name or old_ref_ref

if starting:
infof('Fetching {} from {}', head_ref, head_url)
git_quiet(1, 'fetch', head_url, head_ref)
cls.git_fetch(head_url, head_ref)
git_quiet(1, 'checkout', '-b', tmp_ref,
'FETCH_HEAD')
try:
if starting:
cls.git_fetch(base_url, base_ref)
infof('Rebasing to {} in {}',
base_ref, base_url)
git_quiet(1, 'fetch', base_url, base_ref)
cls.create_rebasing_file(pull, args, old_ref)
# Only run the rebase if we are not continuing with
# a pull rebase that the user finished rebasing using
Expand Down

0 comments on commit 3fb02d1

Please sign in to comment.