All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Tait <git.git@t41t.com>
Cc: git@vger.kernel.org, Erik Faye-Lund <kusmabite@gmail.com>,
	gitster@pobox.com, johannes.schindelin@gmx.de,
	Theo Niessink <theo@taletn.com>
Subject: Re: [PATCH maint 0/3] do not write files outside of work-dir
Date: Wed, 01 Jun 2011 08:31:57 +0200	[thread overview]
Message-ID: <4DE5DCDD.4020303@viscovery.net> (raw)
In-Reply-To: <20110601041439.GH29958@ece.pdx.edu>

Am 6/1/2011 6:14, schrieb Tait:
>> Theo Niessink has uncovered a serious sercurity issue in Git for Windows,
>> where cloning an evil repository can arbitrarily overwrite files outside
>> the repository...
> 
> Filenames starting with C: are not necessarily absolute. Consider
> "c:foo.txt" where c: is the current directory on drive C, or

We have a different notion of "absolute path". This one *is* absolute per
our definition. See below.

> "c:stream1" where c is a single-letter filename in the current directory
> with an alternate data stream such as would be shown by dir /r. The

On my system, this does not create a file in the current directory with an
alternate data stream, but - while the working directory is somewhere on
drive D - a file is created on drive C.

> has_dos_drive_prefix check is overly broad. Maybe this is intentional and
> just needs to be documented. Absolute paths like \\localhost\C$\file.txt
> and \\?\C:\file.txt do seem to be caught, because they start with '\'.
> 
> Microsoft says[1] a path is relative unless:
>   - it begins with "\\"
>   - it begins with a disk designator followed by a directory separator
>   - it begins with a single "\"
> 
> On that basis, has_dos_drive_prefix(path) should be:
>   isalpha(*(path)) && (path)[1] == ':' && is_dir_sep((path)[2])

This is not the definition of "relative path" that we are interested in.
Let $PWD be the current directory. For our purposes, a path $P is relative
if $P and $PWD/$P designate the same file system entry. Otherwise, $P is
an absolute path.

With this definition, the current has_dos_drive_prefix() is good enough.

> However, there are also paths within the NT namespace (as opposed to the
> Win32 namespace, [1] again) that might be considered absolute, or at least
> to which git should not try to write. Examples would be PRN, CONOUT$, AUX,

For our purposes, these names are all relative paths. It's a case of
"Doctor, it hurts when I stick my finger in my eye" if you have a
repository with these names.

Note that git never writes to these files: It always first allocates a
temporary file, eg. nul.123456; but this will already fail because these
special file names are forbidden even when a file extension is attached.

-- Hannes

      reply	other threads:[~2011-06-01  6:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-27 16:00 [PATCH maint 0/3] do not write files outside of work-dir Erik Faye-Lund
2011-05-27 16:00 ` [PATCH 1/3] A Windows path starting with a backslash is absolute Erik Faye-Lund
2011-05-27 16:00 ` [PATCH 2/3] real_path: do not assume '/' is the path seperator Erik Faye-Lund
2011-05-27 16:00 ` [PATCH 3/3] verify_path: consider dos drive prefix Erik Faye-Lund
2011-05-27 18:58   ` Johannes Sixt
2011-05-30  9:32     ` Erik Faye-Lund
2011-05-30 10:58       ` Theo Niessink
2011-05-30 11:17         ` Erik Faye-Lund
2011-06-07  3:46           ` Junio C Hamano
2011-06-07 10:07             ` Erik Faye-Lund
2011-06-07 19:09               ` Erik Faye-Lund
2011-06-07 19:22                 ` Junio C Hamano
2011-06-07 19:32                   ` Erik Faye-Lund
2011-06-07 11:46             ` Theo Niessink
2011-05-30 20:23       ` Johannes Sixt
2011-05-27 17:57 ` [PATCH maint 0/3] do not write files outside of work-dir Junio C Hamano
2011-05-27 18:09   ` Johannes Schindelin
2011-05-27 19:16     ` Junio C Hamano
2011-06-01  4:14 ` Tait
2011-06-01  6:31   ` Johannes Sixt [this message]

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=4DE5DCDD.4020303@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git.git@t41t.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kusmabite@gmail.com \
    --cc=theo@taletn.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.