git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1

From
RHRichard Hansen <rhansen@bbn.com>
Date
Apr 21, 2014, 21:07 UTC
Message-ID
<53558886.5080102@bbn.com>
In-Reply-To
<20140421202454.GA6062@sigill.intra.peff.net>
On 2014-04-21 16:24, Jeff King wrote:
Show 15 quoted lines
> On Mon, Apr 21, 2014 at 03:07:28PM -0400, Richard Hansen wrote:
> 
>> Both bash and zsh subject the value of PS1 to parameter expansion,
>> command substitution, and arithmetic expansion.  Rather than include
>> the raw, unescaped branch name in PS1 when running in two- or
>> three-argument mode, construct PS1 to reference a variable that holds
>> the branch name.  Because the shells do not recursively expand, this
>> avoids arbitrary code execution by specially-crafted branch names such
>> as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.
> 
> Cute. We already disallow quite a few characters in refnames (including
> space, as you probably discovered), and generally enforce that during
> ref transfer. I wonder if we should tighten that more as a precuation.
> It would be backwards-incompatible, but I wonder if things like "$" and
> ";" in refnames are actually useful to people.

That's a tough call. I imagine those that legitimately use '$', ';', or '`' would be annoyed but generally accepting given the security benefit.

I wonder how many repos at sites like GitHub use unusual punctuation in ref names.

Perhaps the additional character restrictions could be controlled via a config option. It would default to the more secure mode but developers/repo admins could relax it where required.

If imposing additional character restrictions is unpalatable, hooks could be used to reject funny branch names in shared repos. But this would require administrator action -- it's not as secure by default.

Show 7 quoted lines
> 
> Did you look into similar exploits with completion? That's probably
> slightly less dire (this one hits you as soon as you "cd" into a
> malicious clone, whereas completion problems require you to actually hit
> <tab>). I'm fairly sure that we miss some quoting on pathnames, for
> example. That can lead to bogus completion, but I'm not sure offhand if
> it can lead to execution.
I have not looked at the completion code.
-Richard
Previous: Jeff KingNext: Michael Haggerty
Message 3 of 11 in “git-prompt.sh: don't put unsanitized branch names in $PS1”
  1. git-prompt.sh: don't put unsanitized branch names in $PS1Richard Hansen, Apr 21, 2014
  2. Jeff KingApr 21, 2014
  3. Richard HansenApr 21, 2014
  4. Michael HaggertyApr 22, 2014
  5. Junio C HamanoApr 22, 2014
  6. Richard HansenApr 22, 2014
  7. Junio C HamanoApr 22, 2014
  8. Junio C HamanoApr 21, 2014
  9. Junio C HamanoApr 21, 2014
  10. Richard HansenApr 21, 2014
  11. git-prompt.sh: don't put unsanitized branch names in $PS1Richard Hansen, Apr 21, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.