Skip to content
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

Missing sanitization of data received from GitHub #197

Closed
jwilk opened this issue Sep 6, 2016 · 8 comments
Closed

Missing sanitization of data received from GitHub #197

jwilk opened this issue Sep 6, 2016 · 8 comments
Assignees
Milestone

Comments

@jwilk
Copy link
Contributor

jwilk commented Sep 6, 2016

git-hub trusts data received from GitHub, and passes it unsanitized to the git command.
Malicious GitHub operators could exploit this to execute arbitrary code.
For example, if GitHub reported the repository name as

--config=core.gitProxy=perl

and the repository URL as

git://-esystem("cowsay pwned > \x2fdev\x2ftty")/

then this would happen:

$ git hub clone https://github.com/sociomantic-tsunami/git-hub
Checking for existing fork / forking...
Fork at https://github.com/jwilk/git-hub
Cloning git://-esystem("cowsay pwned > \x2fdev\x2ftty")/ to --config=core.gitProxy=perl
 _______
< pwned >
 -------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||
Error: git clone --quiet git://-esystem("cowsay pwned > \x2fdev\x2ftty")/ --config=core.gitProxy=perl failed (return code: 128)

With Python before 2.7.9, which didn't verify certificates by default, this bug could be also exploited by man-in-the-middle attackers.

@llucax llucax added this to the v0.10.3 milestone Sep 6, 2016
@llucax
Copy link
Member

llucax commented Sep 6, 2016

Ouch.

@llucax
Copy link
Member

llucax commented Sep 6, 2016

Do you think it would be enough to use -- as a separator for references? What other kind of sanitization do you think it can be performed?

@jwilk
Copy link
Contributor Author

jwilk commented Sep 6, 2016

Using -- is probably a good idea, yes.
Apart from this: repository name is used as default destination directory, so git-hub should ensure that it doesn't contain / characters and that it's not equal to . or ...

@jwilk
Copy link
Contributor Author

jwilk commented Sep 8, 2016

Some protocols (such as git-remote-ext) can execute arbitrary code found in the URL.
git-hub should validate that the URL returned by GitHub uses a sane protocol.
It might be also worth verifying that the URL actually points to GitHub, and not some other host.

@jwilk
Copy link
Contributor Author

jwilk commented Sep 30, 2016

CVE-2016-7793 (for missing validation of repository URL) and CVE-2016-7794 (for missing validation of repository name) were assigned to this bug.

leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 5, 2016
Without using the option terminator, a malicious server could inject
undesired options into the command-line (like
`--config=core.gitProxy=perl`) to then use the URL to execute arbitrary
code.

This should partially fix sociomantic-tsunami#197 (git-remote-ext remains as an attack
vector).
@leandro-lucarella-sociomantic
Copy link
Contributor

OK, #203 should fix the non git-remote-ext issues, as the Github server can't inject git options anymore.

leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 5, 2016
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 completely fix sociomantic-tsunami#197, and with it both CVE-2016-7793 and
CVE-2016-7794.
@leandro-lucarella-sociomantic
Copy link
Contributor

And #204 should complete the fix.

leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 5, 2016
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 completely fix sociomantic-tsunami#197, and with it both CVE-2016-7793 and
CVE-2016-7794.
leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 6, 2016
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 completely fix sociomantic-tsunami#197, and with it both CVE-2016-7793 and
CVE-2016-7794.
leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 6, 2016
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.
leandro-lucarella-sociomantic added a commit to leandro-lucarella-sociomantic/git-hub that referenced this issue Oct 6, 2016
Without using the option terminator, a malicious server could inject
undesired options into the command-line (like
`--config=core.gitProxy=perl`) to then use the URL to execute arbitrary
code.

This completes the fix for sociomantic-tsunami#197.
nemanja-boric-sociomantic pushed a commit that referenced this issue Oct 7, 2016
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 #197.
nemanja-boric-sociomantic pushed a commit that referenced this issue Oct 7, 2016
Without using the option terminator, a malicious server could inject
undesired options into the command-line (like
`--config=core.gitProxy=perl`) to then use the URL to execute arbitrary
code.

This completes the fix for #197.
@leandro-lucarella-sociomantic
Copy link
Contributor

I'll release v0.10.3 soon, but I'll use it for a day or so as a way to test it first. If more people can give the branch v0.10.x a try too, it would be helpful.

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

No branches or pull requests

3 participants