Re: Bug#347548: DOMAIN CHECK constraint bypassed

Started by Peter Eisentrautabout 20 years ago5 messagesbugs
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

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/

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)

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

#3Neil Conway
neilc@samurai.com
In reply to: Peter Eisentraut (#1)

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#3)

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

#5Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#4)

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