| Subject: | Type-specific quoting functions considered harmful (with bonus SQL-injection) |
quote.c contains a number of functions for different types; most of
these are broken, as follows:
quote_geom rejects almost all valid geometric values, due to too-strict
limitations on allowed characters (no provision for -, ., E, etc. --
coordinates are floating-point values)
quote_path, quote_circle likewise
quote_bool does what it is supposed to, but the logic is in completely
the wrong place (see below).
quote_integer seems, fortunately, to be dead code - if it were ever used
it would break all kinds of stuff.
and, to cap it off, the use of null_quote on integer fields can lead to
SQL injection:
$s=$d->prepare(q[select ? where 1=?], { pg_server_prepare => 0 });
$s->bind_param(2,undef,SQL_INTEGER);
$s->execute(1,"2; drop table x;");
because nothing checks that the value is actually an integer.
Putting type-specific logic like that of quote_bool into a quote_*
function only handles half the job; it leaves out the question of what
to do if a parameter of the same type is passed directly to PQexecParams
or PQexecPrepared. This leads to, for example, boolean values other than
exactly "0" or "1" being sometimes rejected and sometimes accepted, e.g.
$s=$d->prepare(q[select 1 where ? or ?=1], { pg_server_prepare => 0 });
$s->bind_param(1,undef,SQL_BOOLEAN);
$s->execute("f",2); # errors
$s=$d->prepare(q[select 1 where ?], { pg_server_prepare => 0 });
$s->bind_param(1,undef,SQL_BOOLEAN);
$s->execute("f"); # "f" treated as false
(also, quote_bool treats perl "true" values, such as "0e0" or "0 but
true" as though they were false, while these produce errors if passed as
real parameters)
This whole thing needs a redesign, not just patching up the existing code.