Re: is_absolute_path incorrect on Windows - Mailing list pgsql-hackers

On Fri, Apr 9, 2010 at 16:02, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
> Magnus Hagander <magnus@hagander.net> wrote:
>
>> it considers the following to be an absolute path:
>> * Anything that starts with /
>> * Anything that starst with \
>
> These aren't truly absolute, because the directory you find will be
> based on your current work directory's drive letter; however, if the
> point is to then check whether it falls under the current work
> directory, even when an absolute path is specified, it works.

That is true. However, since we have chdir():ed into our data
directory, we know which drive we are on. So I think we're safe.


>> * Anything alphanumerical, followed by a colon, followed by either
>> / or \
>
> I assume we reject anything where what precedes the colon doesn't
> match the current drive's designation?

Define reject? We're just answering the question "is absolute path?".
It's then up to the caller. For example, in the genfiles function, we
will take the absolute path and compare it to the path specified for
the data directory, to make sure we can't go outside it.


>> However, it misses the case with for example E:foo, which is a
>> perfectly valid path on windows. Which isn't absolute *or*
>> relative - it's relative to the current directory on the E: drive.
>
>> This function is used in the genfile.c functions to read and list
>> files by admin tools like pgadmin - to make sure we can only open
>> files that are in our own data directory - by making sure they're
>> either relative, or they're absolute but rooted in our own data
>> directory. (It rejects anything with ".." in it already).
>
> Well, if that's a good idea, then you would need to reject anything
> specifying a drive which doesn't match the drive of the data
> directory.  Barring the user from accessing directories on the
> current drive which aren't under the data directory on that drive,
> but allowing them to access any other drive they want, is just
> silly.

Yes. That's what the code does - once it's determined that it's an
absolute directory, it will compare the start of it to the data
directory. This will obviously not match if the data directory is on a
different drive.


> It does raise the question of why we need to check this at all,
> rather than counting on OS security to limit access to things which
> shouldn't be seen.

That is a different question, of course. For reading, it really
should. But there was strong opposition to that when the functions
were added, so this was added as an extra security check.

This is why we're not treating it as a security problem. The
callpoints require you to have superuser, so this is really just a way
to make it a bit harder to do things wrong. There are other ways to
get to the information, so it's not a security issue.

It's more about preventing you from doing the wrong thing by mistake.
Say a "\copy foo e:foo.csv" instead of "e:/foo.csv", that might
overwrite the wrong file by mistake - since the path isn't fully
specified.

-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/