From: Richard Hansen <rhansen@bbn.com>
To: Junio C Hamano <gitster@pobox.com>,
Michael Haggerty <mhagger@alum.mit.edu>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org, sitaramc@gmail.com
Subject: Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
Date: Tue, 22 Apr 2014 14:38:18 -0400 [thread overview]
Message-ID: <5356B71A.6070500@bbn.com> (raw)
In-Reply-To: <xmqqy4yx5knw.fsf@gitster.dls.corp.google.com>
On 2014-04-22 13:38, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> While we're at it, I think it would be prudent to ban '-' at the
>> beginning of reference name segments. For example, reference names like
>>
>> refs/heads/--cmd=/sbin/halt
>> refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
>>
>> are currently both legal, but I think they shouldn't be.
>
> I think we forbid these at the Porcelain level ("git branch", "git
> checkout -b" and "git tag" should not let you create "-aBranch"),
> while leaving the plumbing lax to allow people experimenting with
> their repositories.
>
> It may be sensible to discuss and agree on what exactly should be
> forbidden (we saw "leading dash", "semicolon and dollar anywhere"
> so far in the discussion)
Also backquote anywhere.
> and plan for transition to forbid them
> everywhere in a next big version bump (it is too late for 2.0).
Would it be acceptable to have a config option to forbid these in a
non-major version bump? Does parsing config files add too much overhead
for this to be feasible?
If it's OK to have a config option, then here's one possible transition
path (probably flawed, but my intent is to bootstrap discussion):
1. Add an option to forbid dangerous characters. The option defaults
to disabled for compatibility. If the option is unset, print a
warning upon encountering a ref name that would be forbidden.
2. Later, flip the default to enabled.
3. Later, in the weeks/months leading up to the next major version
release, print the warning even if the config option is set to
disabled.
Thanks,
Richard
next prev parent reply other threads:[~2014-04-22 18:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-21 19:07 [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1 Richard Hansen
2014-04-21 20:24 ` Jeff King
2014-04-21 21:07 ` Richard Hansen
2014-04-22 8:38 ` Michael Haggerty
2014-04-22 17:38 ` Junio C Hamano
2014-04-22 18:38 ` Richard Hansen [this message]
2014-04-22 19:47 ` Junio C Hamano
2014-04-21 22:23 ` Junio C Hamano
2014-04-21 22:33 ` Junio C Hamano
2014-04-21 22:58 ` Richard Hansen
2014-04-21 23:53 ` [SECURITY PATCH v2] " Richard Hansen
-- strict thread matches above, loose matches on Subject: below --
2014-04-24 18:40 [SECURITY PATCH] " Gábor Szeder
2014-04-25 7:37 ` Simon Oosthoek
2014-04-25 16:39 ` Richard Hansen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5356B71A.6070500@bbn.com \
--to=rhansen@bbn.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=peff@peff.net \
--cc=sitaramc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.