Bug#840014: webcheckout: missing URL sanitization

2018-02-24 Thread Paul Wise
On Sat, 2018-02-24 at 18:46 +0100, Jakub Wilk wrote: > ">=" compares numerically even when arguments are strings, so the int() > calls aren't needed here. > > More importantly, this will break when Git 3.0 is released, because > int($minor) >= 12 will be no longer true. Fixed in git:

Bug#840014: webcheckout: missing URL sanitization

2018-02-24 Thread Jakub Wilk
if (int($major) >= 2 && int($minor) >= 12) { ">=" compares numerically even when arguments are strings, so the int() calls aren't needed here. More importantly, this will break when Git 3.0 is released, because int($minor) >= 12 will be no longer true. -- Jakub Wilk

Bug#840014: webcheckout: missing URL sanitization

2018-02-22 Thread Paul Wise
Control: tags -1 + fixed-upstream patch upstream Control: forwarded -1 http://source.myrepos.branchable.com/?p=source.git;a=commitdiff;h=40a3df21c73f1bb1b6915cc6fa503f50814664c8 On Thu, 2018-02-22 at 22:09 +0100, Jakub Wilk wrote: > Uppercase letter should be allowed in usernames and domains, I

Bug#840014: webcheckout: missing URL sanitization

2018-02-22 Thread Jakub Wilk
$git_url !~ m{^(?:(?:https?|git|ssh):[^:]|(?:[-_.a-z0-9]+@)?[-_.a-z0-9]+:(?:[^:]|$))}) { Uppercase letter should be allowed in usernames and domains, I guess? This regexp matches "foo://bar"; but this URL would make git execute remote helper "foo", which might be unsafe. I suggest replacing

Bug#840014: webcheckout: missing URL sanitization

2018-02-18 Thread Paul Wise
On Sun, 2018-02-18 at 22:27 +0100, Jakub Wilk wrote: > You don't need trailing undef here. Tested, removed. > SSH protocol has an alternative scp-like syntax: Added, hope I got the regex right. > There are also two syntaxes for local repositories, although I think > neither should be

Bug#840014: webcheckout: missing URL sanitization

2018-02-18 Thread Paul Wise
On Sun, 2018-02-18 at 22:27 +0100, Jakub Wilk wrote: > You don't need trailing undef here. Tested, removed > SSH protocol has an alternative scp-like syntax: Added, hope I got the regex right. > There are also two syntaxes for local repositories, although I think > neither should be allowed.

Bug#840014: webcheckout: missing URL sanitization

2018-02-18 Thread Jakub Wilk
+my ($major, $minor, undef) = split(/\./, $git_version); You don't need trailing undef here. (The number of components is a git version varies between 3 and 4, so you can't make the number of items of the left side always match anyway.) + if ($git_unsafe && $git_url !~

Bug#840014: webcheckout: missing URL sanitization

2018-02-11 Thread Paul Wise
On Sun, 2018-02-11 at 22:04 +0800, Paul Wise wrote: > I think I will check the git version and apply the manual > whitelisting only for versions of git older than 2.12. Attached my proposed patch for the git issue. -- bye, pabs https://wiki.debian.org/PaulWise From

Bug#840014: webcheckout: missing URL sanitization

2018-02-11 Thread Paul Wise
On Sun, 2018-02-11 at 14:09 +0100, Jakub Wilk wrote: > For Git (>= 2.12), you can set GIT_PROTOCOL_FROM_USER=0 in > environment. > Quoting git(1): "this is useful [...] for programs which feed > potentially-untrusted URLS to git commands". Ah, I missed that addition. > If you want to support

Bug#840014: webcheckout: missing URL sanitization

2018-02-11 Thread Jakub Wilk
* Paul Wise , 2018-02-09, 09:56: Users might need to re-enable git-remote-ext for their own purposes, so this needs to be fixed in webcheckout. How would you suggest doing that? * Blacklist ext:: * Whitelist good remote protocols For Git (>= 2.12), you can set

Bug#840014: webcheckout: missing URL sanitization

2018-02-08 Thread Paul Wise
On Thu, 2018-02-08 at 21:44 +0100, Jakub Wilk wrote: > It's hard to tell whether they agree, because disabling git-remote-ext > by default is not documented AFAICT. See bug #867699. Thanks for the pointer. > Users might need to re-enable git-remote-ext for their own purposes, so > this needs

Bug#840014: webcheckout: missing URL sanitization

2018-02-08 Thread Jakub Wilk
* Paul Wise , 2018-02-07, 16:59: $ webcheckout /path/to/badgit.html git clone ext::sh -c cowsay% pwned% >% /dev/tty I consider this particular attack to be a bug in git and the git authors seem to agree with me because it is blocked in sid. It's hard to tell whether they

Bug#840014: webcheckout: missing URL sanitization

2018-02-07 Thread Paul Wise
On Fri, 7 Oct 2016 18:27:12 +0200 Jakub Wilk wrote: > $ webcheckout /path/to/badgit.html > git clone ext::sh -c cowsay% pwned% >% /dev/tty I consider this particular attack to be a bug in git and the git authors seem to agree with me because it is blocked in sid. Do you think this should be

Bug#840014: webcheckout: missing URL sanitization

2016-10-07 Thread Jakub Wilk
Package: myrepos Version: 1.20160123 Tags: security webcheckout passes the extracted URL to "git clone", without any sanitization. Malicious website operators or MitM attackers could exploit it for arbitrary code execution. PoC: $ webcheckout /path/to/badgit.html git clone ext::sh -c