public inbox for git@vger.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$(echo .)net \
    --cc=git.git@t41t$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=johannes.schindelin@gmx$(echo .)de \
    --cc=kusmabite@gmail$(echo .)com \
    --cc=theo@taletn$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox