Thread: Re: [HACKERS] Database names with spaces

Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> > Looks like a bug.  Added to TODO list.
>> 
>> >> I see a todo item
>> >> * Views with spaces in view name fail when referenced
>> >> 
>> >> I have another one for you:
>> >> * Databases with spaces in name fail to be created and destroyed despite
>> >> responses to the contrary.
> 
>> IIRC, createdb and destroydb use "cp -r" and "rm -r" respectively.
>> Lack of careful quoting in the system calls is probably what's
>> causing the problem here.
>> 
>> However, I wonder if it wouldn't be a better idea to forbid funny
>> characters in things that will become Unix filenames.  In particular,
>> something like CREATE DATABASE "../../../something" could have real
>> bad consequences...
>
>I just tried it:
>
>        test=> create database "../../pg_hba.conf"
>        test-> \g
>        ERROR:  Unable to locate path '../../pg_hba.conf'
>                This may be due to a missing environment variable in the server
>
>Seems we are safe.

(This is my first time going through the code, so I could be getting alot of
things wrong, but here's what I see...)

The function createdb in backend/commands/dbcommands.c (I assume this is right 
because it seems to do the correct thing and actually define the above error
message) tries to get the path to the database using ExpandDatabasePath on
either dbpath/dbname or just dbname depending on whether dbpath is null (or
same as dbname).

ExpandDatabasePath in backend/utils/misc/database.c seems to assume that anything
that has the separator character ('/' i would assume) is of the form
environmentvariable/<rest> which seems to be rewritten into
<value of environment variable>/base/<rest>
So with your example it fails because it can't find the environment variable
'..'  (although if you had one, it might actually attempt to put it wherever
that would point)

It then makes a command and systems:
COPY_CMD DataDir/base/template1/ <return from ExpandDatabasePath>

When i tried to do a:create database "`a.sh`"  on a little shell script in the PATH, it decided
to copy the stuff from template1 into my data/base directory.  The shell
script touches a file in tmp, which was touched.

It seems like the current implementation would let someone run a command in
backticks that is in the postgres user's path as long as there are no
directory name separators in the command.

Stephan Szabo

Can someone comment on this?


> 
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> >> > Looks like a bug.  Added to TODO list.
> >> 
> >> >> I see a todo item
> >> >> * Views with spaces in view name fail when referenced
> >> >> 
> >> >> I have another one for you:
> >> >> * Databases with spaces in name fail to be created and destroyed despite
> >> >> responses to the contrary.
> > 
> >> IIRC, createdb and destroydb use "cp -r" and "rm -r" respectively.
> >> Lack of careful quoting in the system calls is probably what's
> >> causing the problem here.
> >> 
> >> However, I wonder if it wouldn't be a better idea to forbid funny
> >> characters in things that will become Unix filenames.  In particular,
> >> something like CREATE DATABASE "../../../something" could have real
> >> bad consequences...
> >
> >I just tried it:
> >
> >        test=> create database "../../pg_hba.conf"
> >        test-> \g
> >        ERROR:  Unable to locate path '../../pg_hba.conf'
> >                This may be due to a missing environment variable in the server
> >
> >Seems we are safe.
> 
> (This is my first time going through the code, so I could be getting alot of
> things wrong, but here's what I see...)
> 
> The function createdb in backend/commands/dbcommands.c (I assume this is right 
> because it seems to do the correct thing and actually define the above error
> message) tries to get the path to the database using ExpandDatabasePath on
> either dbpath/dbname or just dbname depending on whether dbpath is null (or
> same as dbname).
> 
> ExpandDatabasePath in backend/utils/misc/database.c seems to assume that anything
> that has the separator character ('/' i would assume) is of the form
> environmentvariable/<rest> which seems to be rewritten into
> <value of environment variable>/base/<rest>
> So with your example it fails because it can't find the environment variable
> '..'  (although if you had one, it might actually attempt to put it wherever
> that would point)
> 
> It then makes a command and systems:
> COPY_CMD DataDir/base/template1/ <return from ExpandDatabasePath>
> 
> When i tried to do a:
>  create database "`a.sh`"  on a little shell script in the PATH, it decided
> to copy the stuff from template1 into my data/base directory.  The shell
> script touches a file in tmp, which was touched.
> 
> It seems like the current implementation would let someone run a command in
> backticks that is in the postgres user's path as long as there are no
> directory name separators in the command.
> 
> Stephan Szabo
> 
> ************
> 


--  Bruce Momjian                        |  http://www.op.net/~candle pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026
 

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Can someone comment on this?

Peter fixed all that stuff for 7.0.  No backticks allowed in database
paths anymore ;-).  I do recall having tested a DB name with a space
in it just recently...
        regards, tom lane

On Wed, May 31, 2000 at 07:08:57PM -0400, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Can someone comment on this?
> 
> Peter fixed all that stuff for 7.0.  No backticks allowed in database
> paths anymore ;-).  

Hey, Peter does a _lot_ but I get credit for excluding backticks from
DB path, on March 7. (Or is there seperate code to deal with the dbname
itself? That'd be Peter, then)

Ross
(Hmm, why do I suddenly feel like the younger brother, who's trophy
for 'honorable mention: state bowling championship' is completely
overshadowed on the fireplace mantel by his older brother's gazillion
track and football trophies? ;-)

"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
>> Peter fixed all that stuff for 7.0.  No backticks allowed in database
>> paths anymore ;-).  

> Hey, Peter does a _lot_ but I get credit for excluding backticks from
> DB path, on March 7.

Humblest apologies --- I remembered Peter complaining about the space-
in-DB-name issue, and thought he'd done all the related work.

Credit is a slippery thing when there are so many people working on
the code... but I don't think anyone wants to slight anyone else's
contribution.
        regards, tom lane

On Thu, Jun 01, 2000 at 01:38:30AM -0400, Tom Lane wrote:
> 
> > Hey, Peter does a _lot_ but I get credit for excluding backticks from
> > DB path, on March 7.
> 
> Humblest apologies --- I remembered Peter complaining about the space-
> in-DB-name issue, and thought he'd done all the related work.
> 
> Credit is a slippery thing when there are so many people working on
> the code... but I don't think anyone wants to slight anyone else's
> contribution.

Hey, it's truly minor, no slight felt. I hoped to be humorous, so as
not to appear petty, but it's too late at night, as well.  To bed!

Later,
Ross
Well, it helps to be a petty nitpicker when dealing with security issues,
right?