Thread: Re: Bug#347548: DOMAIN CHECK constraint bypassed
This bug was reported to Debian. Comments?
Tim Southerwood wrote:
> Package: postgresql-8.1
> Version: 8.1.2-1
> Severity: important
>
> DOMAIN CHECK constraint is bypassable when inserting rows using
> perl/DBD::Pg AND prepare/execute semantics AND using bind values.
>
> This is serious as data integrity rules are not consistently
> enforced.
>
> To reproduce:
>
> Create a database, we will use the name "photostore" for this
> example. Run the following SQL through psql:
>
> -- Cut
> CREATE DOMAIN absdirpath AS text
> CHECK(
> VALUE ~ '^[[:print:]]+$' AND
> VALUE ~ '^/'
> );
>
> CREATE TABLE image
> (
> basedir absdirpath NOT NULL
> ) WITH OIDS;
> -- Cut
>
> Now try the following perl program (you will need to adjust
> connection parameters:
>
> # Cut
> #!/usr/bin/perl
>
> use strict;
> use warnings;
>
> use DBI;
>
> my $res;
> # Change to suit your database server
> my $dbh = DBI->connect("dbi:Pg:dbname=photostore", '', '',
> {AutoCommit => 1, RaiseError => 0, PrintError => 1});
>
> die "Cannot open database connection" unless defined $dbh;
>
> $res = $dbh->do("insert into image (basedir) values ('/tmp')");
> if ($res)
> {
> print "Insert string was allowed, OK\n";
> }
> else
> {
> print "Insert string was disallowed, error\n";
> }
>
> $res = $dbh->do("insert into image (basedir) values ('')");
> if ($res)
> {
> print "Insert empty string was allowed, error\n";
> }
> else
> {
> print "Insert empty string was disallowed, OK\n";
> }
>
> my $sth=$dbh->prepare("insert into image (basedir) values (?)");
> $res = $sth->execute("");
> if ($res)
> {
> print "Insert empty string via bind was allowed, error\n";
> }
> else
> {
> print "Insert empty string via bind was disallowed, OK\n";
> }
>
> $sth=$dbh->prepare("insert into image (basedir) values (?)");
> $res = $sth->execute(undef);
> if ($res)
> {
> print "Insert NULL via bind was allowed, error\n";
> }
> else
> {
> print "Insert NULL via bind was disallowed, OK\n";
> }
>
> $dbh->disconnect();
> # Cut
>
> The output I get is:
>
> # Cut
>
> Insert string was allowed, OK
> DBD::Pg::db do failed: ERROR: value for domain absdirpath violates
> check constraint "absdirpath_check"
> Insert empty string was disallowed, OK
> Insert empty string via bind was allowed, error
> DBD::Pg::st execute failed: ERROR: null value in column "basedir"
> violates not-null constraint
> Insert NULL via bind was disallowed, OK
>
> # Cut
>
> You can clearly see that inserting the empty string via do("INSERT
> ...") is correctly rejected, but performing the same insert via
> prepare/execute with bind values succeeds.
>
> Further verifcation: Connect to the database via psql and try some
> selects. Here's my example:
>
> -- Cut
>
> photostore=> SELECT basedir from image;
> basedir
> ---------
> /tmp
>
> (2 rows)
>
> photostore=> SELECT length(basedir) from image;
> length
> --------
> 4
> 0
> (2 rows)
>
> -- Cut
>
> We have one row which should be impossible to insert.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes:
> This bug was reported to Debian. Comments?
These are all known issues. The implementation of domains really needs
rather a lot of work :-(.
One thing we might think about is having domain types use a special set
of input functions rather than just linking directly to the base-type
functions (kinda like arrays use array_in, etc). These functions could
then be charged with applying the appropriate constraint checks after
invoking the correct base-type input function. I'm not sure about
performance issues (ie, how to avoid repetitive lookups and memory
leaks) but at least we'd only need to solve it in one place and not
several.
regards, tom lane
On Sat, 2006-01-28 at 20:17 +0100, Peter Eisentraut wrote: > This bug was reported to Debian. Comments? AFAICS I fixed this a few weeks ago (post-8.1.2): http://archives.postgresql.org/pgsql-committers/2006-01/msg00209.php http://archives.postgresql.org/pgsql-patches/2006-01/msg00139.php I get the following results with the test script on REL8_1_STABLE: Insert string was allowed, OK DBD::Pg::db do failed: ERROR: value for domain absdirpath violates check constraint "absdirpath_check" Insert empty string was disallowed, OK DBD::Pg::st execute failed: ERROR: value for domain absdirpath violates check constraint "absdirpath_check" Insert empty string via bind was disallowed, OK DBD::Pg::st execute failed: ERROR: null value in column "basedir" violates not-null constraint Insert NULL via bind was disallowed, OK (FWIW I certainly agree with Tom that the domain implementation needs a fair bit of work.) -Neil
Neil Conway <neilc@samurai.com> writes:
> On Sat, 2006-01-28 at 20:17 +0100, Peter Eisentraut wrote:
>> This bug was reported to Debian. Comments?
> AFAICS I fixed this a few weeks ago (post-8.1.2):
You only fixed the bind-parameter case, though, no? The problem is
still rampant in the PLs.
regards, tom lane
On Mon, 2006-01-30 at 20:45 -0500, Tom Lane wrote: > You only fixed the bind-parameter case, though, no? The problem is > still rampant in the PLs. Right: the bug report was specific to the bind parameter case. Domain constraints should also be checked before returning values of a domain type from all of the procedural languages, and before casting a value to a domain type in PL/PgSQL. This can be done by adding constraint checks to each PL individually, like this patch for PL/PgSQL's return value: http://archives.postgresql.org/pgsql-patches/2006-01/msg00107.php (The patch hasn't been applied because there is some additional infrastructure needed to get good performance.) We could do similar work for each PL. That actually wouldn't be too bad: adding the necessary domain constraint information (plus sinval support) to the typcache would allow most of the code to be shared. Or we could refactor the code more cleanly, as Tom suggests -- it's not clear to me quite how to do that without accepting a performance hit, though... -Neil