> All I really wanted to do is fix TODO item
> * database names with spaces fail
> but that is already taken care of, they work fine. Please check it off.
> Meanwhile, database names with single quotes in names don't work very well
> at all, and because of shell quoting rules this can't be fixed, so I put
> in error messages to that end.

That seems to be a bit heavy handed; why bother disallowing things in
the backend because some (small number of) shell-based tools have
trouble as clients? I'd prefer filtering that at the client end, and
allowing capable clients to do whatever they please.

There is a related issue which afaik no one has addressed yet: the
permissions ACLs are stored as a string with a format like
"accountname=permissions" (doing this from memory, so the details may
be wrong) but with quoting allowed for table names and user names one
could embed an equals sign into an account or group name and muck with
permissions. I haven't looked at the code in a long time, but was
thinking about recoding ACLs as a two-field type to enforce an
unambigous interpretation of the two fields. Interested??

                        - Thomas

-- 
Thomas Lockhart				lockhart@alumni.caltech.edu
South Pasadena, California

Tue, 14 Dec 1999 14:21:35 +0000
Thomas Lockhart <lockhart@alumni.caltech.edu>
Re: [PATCHES] createdb/dropdb fixes
Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
>> Meanwhile, database names with single quotes in names don't work very well
>> at all, and because of shell quoting rules this can't be fixed, so I put
>> in error messages to that end.

> That seems to be a bit heavy handed; why bother disallowing things in
> the backend because some (small number of) shell-based tools have
> trouble as clients? I'd prefer filtering that at the client end, and
> allowing capable clients to do whatever they please.

No, you're missing the point: the backend itself uses shell escapes
for some whole-database functions.  IIRC, database creation is done with
something like
	system("cp -r base/template1 base/newdb");
So shell metacharacters in database names are Bad News.  We need to
put in a filter that will prevent appearances of / | ` etc in DB names.
I assume that's what Peter was doing.

I think we may have some bugs with metacharacters in table names (which
become filenames) as well, but haven't really pushed on it.

> thinking about recoding ACLs as a two-field type to enforce an
> unambigous interpretation of the two fields. Interested??

Seems like a good idea.

			regards, tom lane

Tue, 14 Dec 1999 11:45:49 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: [PATCHES] createdb/dropdb fixes 
On 1999-12-14, Thomas Lockhart mentioned:

> That seems to be a bit heavy handed; why bother disallowing things in
> the backend because some (small number of) shell-based tools have
> trouble as clients? I'd prefer filtering that at the client end, and

It's really about statements like this:

	snprintf(buf, sizeof(buf), "rm -rf '%s'", path);

There is no way around disallowing single-quotes unless you double quote
the argument and be very careful with the escaping. Of course this
particular case might as well use unlink(), but there is a recursive copy
of the template1 dir which would take a little more work (opendir(),
etc.). At that point we could lift that restriction.

> permissions. I haven't looked at the code in a long time, but was
> thinking about recoding ACLs as a two-field type to enforce an
> unambigous interpretation of the two fields. Interested??

I've been puzzled about this for a long time, is there a reason this is
stored as an array at all? Why not use tuples like
	aclperm		char?
	aclrelation	oid
	aclentity	oid	/* user or group sysid */
	aclisgroup	bool	/* is it a user or group? */

And then it looks like this:
aclperm|aclrel|acluser|aclisgroup
-------+------+-------+----------
R      |177777|    100|f
W      |177777|    100|f
R      |177777|    120|f
R      |188888|      5|t

That's much cleaner. GRANT and REVOKE would be reduced to simple
insert/delete equivalents. I'm not sure how the actual authentication code
would like that overheadwise, though.

A related issue is pg_group, which I'm currently working on. Those arrays
are killing me. A simple user/group associating relation would be much
nicer.

-- 
Peter Eisentraut                  Sernanders vï¿½g 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Wed, 15 Dec 1999 00:01:39 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: [PATCHES] createdb/dropdb fixes
Peter Eisentraut <peter_e@gmx.net> writes:
> It's really about statements like this:

> 	snprintf(buf, sizeof(buf), "rm -rf '%s'", path);

> There is no way around disallowing single-quotes unless you double quote
> the argument and be very careful with the escaping.

Yes.  In fact, I'd argue for filtering the names more heavily than that;
just to take a for-example, Bad Things would ensue if we accepted a
database name of "..".

It is easy to devise cases in which accepting leading "." or embedded "/"
leads to disaster; if you think those are OK, allow me to destroy your
installation for you ;-).  I haven't yet thought of a way to cause
trouble with a back-quote in a DB name (given that single quotes are
disallowed) ... but I bet some enterprising hacker can find one.

Beyond the bare minimum security issues, I also think we should take
pity on the poor dbadmin who may have to be looking at these
subdirectories or filenames.  Is it really a good idea to allow carriage
returns or other control characters in file/directory names?  Is it
even a good idea to allow spaces?  I don't think so.  If we were not
using these names for Unix file/dir names then we could allow anything
we felt like --- but since we are using them that way, I think that the
safest path is to only allow things that are going to look like ordinary
file names when used in Unix shell commands.  Otherwise there's still a
big chance of trouble if the dbadmin gets a little bit careless.

> Of course this particular case might as well use unlink(),

Not unless your system's unlink is much different from mine's...

			regards, tom lane

Tue, 14 Dec 1999 18:44:15 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: [PATCHES] createdb/dropdb fixes 
Here's another anomaly I've run across in porting the Ars Digita
Community System web development toolkit from Oracle to Postgres:

In oracle, if we do:

SQL> create table foo(i integer, j integer);

Table created.

then select like this, we get no rows returned:

SQL> select i, count(*) from foo group by i;

no rows selected

In postgres, the same select on the same empty table yields:

test=> select i, count(*) from foo group by i;
i|count
-+-----
 |    0
(1 row)

test=> 

Which is correct?  It's the count() causing the row to be output,
apparently PostgreSQL feels obligated to return at least one 
value for the aggragate and since there are no groups has to
invent one.  I assume this is wrong, and that Oracle's right, but
haven't dug through Date's book to verify.



- Don Baccus, Portland OR <dhogaza@pacifier.com>
  Nature photos, on-line guides, Pacific Northwest
  Rare Bird Alert Service and other goodies at
  http://donb.photo.net.

Tue, 14 Dec 1999 16:11:39 -0800
Don Baccus <dhogaza@pacifier.com>
Bug or feature?  select, count(*), group by and empty tables
At 04:11 PM 12/14/99 -0800, Don Baccus wrote:
>Here's another anomaly I've run across in porting the Ars Digita
>Community System web development toolkit from Oracle to Postgres:

(always returning a row for a select count(*) ... group by query
 even if there aren't any groups)

OK, I've gotten the latest sources with the bright idea of digging
around, and in nodeAgg.c the routine ExecAgg.c has been somewhat
rewritten, with comments that make it clear that this bug's already
been fixed.

I should build myself a latest version so I can filter out non-problems
before reporting them, sorry...



- Don Baccus, Portland OR <dhogaza@pacifier.com>
  Nature photos, on-line guides, Pacific Northwest
  Rare Bird Alert Service and other goodies at
  http://donb.photo.net.

Tue, 14 Dec 1999 16:32:55 -0800
Don Baccus <dhogaza@pacifier.com>
Re: [HACKERS] Bug or feature?  select, count(*), group by and
	empty tables
Don Baccus <dhogaza@pacifier.com> writes:
> (always returning a row for a select count(*) ... group by query
>  even if there aren't any groups)

Yah: if you have aggregates and no GROUP, for empty input you should
get one row out with "default" results (0 for COUNT, null for most other
aggregates).  But for GROUP mode, no rows in should yield no rows out,
aggregates or no.  It took a fair amount of arguing before everyone was
convinced that that is the correct interpretation of the spec ;-),
which is why it's only been fixed recently.

> OK, I've gotten the latest sources with the bright idea of digging
> around, and in nodeAgg.c the routine ExecAgg.c has been somewhat
> rewritten, with comments that make it clear that this bug's already
> been fixed.
> I should build myself a latest version so I can filter out non-problems
> before reporting them, sorry...

Not a problem.  Bug reports on the latest release are fair game.
If it's already been fixed in current sources, whoever fixed it
will surely take pleasure in telling you so...

			regards, tom lane

Wed, 15 Dec 1999 00:52:15 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Bug or feature? select, count(*),
	group by and empty tables
On 1999-12-14, Tom Lane mentioned:

> Yes.  In fact, I'd argue for filtering the names more heavily than that;
> just to take a for-example, Bad Things would ensue if we accepted a
> database name of "..".

My rm refuses to remove '..' and '.'.

> It is easy to devise cases in which accepting leading "." or embedded "/"
> leads to disaster; if you think those are OK, allow me to destroy your
> installation for you ;-).  I haven't yet thought of a way to cause
> trouble with a back-quote in a DB name (given that single quotes are
> disallowed) ... but I bet some enterprising hacker can find one.

The slash problem will disappear (I hope) when I fix that alternate
location issue.

> Beyond the bare minimum security issues, I also think we should take
> pity on the poor dbadmin who may have to be looking at these
> subdirectories or filenames.  Is it really a good idea to allow carriage
> returns or other control characters in file/directory names?  Is it
> even a good idea to allow spaces?  I don't think so.  If we were not

Spaces why not? I use spaces all the time in filenames. But perhaps we
should make a definite list of things we won't allow, such as dots (.),
slashes (/), tildes (~), etc., and everything below ASCII 32. But limiting
it to a finite list of characters would really be a blow to people using
other character sets.

> > Of course this particular case might as well use unlink(),
> 
> Not unless your system's unlink is much different from mine's...

Is it just me or is your system intentionally designed to be different
from anybody elses? :) Last time I checked rm does call unlink. Relying on
sh-utils sort of commands has its own set of problems. For example in the
6.5 source, run the postmaster on a terminal, revoke all permissions on a
database directory (chmod a-rwx testdb) and try to drop it. My rm will
prompt on the postmaster terminal for verification while the whole thing
hangs ...

-- 
Peter Eisentraut                  Sernanders vï¿½g 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden



Fri, 17 Dec 1999 01:31:23 +0100 (CET)
Peter Eisentraut <peter_e@gmx.net>
Re: [HACKERS] Re: [PATCHES] createdb/dropdb fixes 