Re: Re: Escaping strings for inclusion into SQL queries

Lists: pgsql-hackers
From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Escaping strings for inclusion into SQL queries
Date: 2001-08-22 16:11:06
Message-ID: tg7kvwqdlx.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

It has come to our attention that many applications which use libpq
are vulnerable to code insertion attacks in strings and identifiers
passed to these applications. We have collected some evidence which
suggests that this is related to the fact that libpq does not provide
a function to escape strings and identifiers properly. (Both the
Oracle and MySQL client libraries include such a function, and the
vast majority of applications we examined are not vulnerable to code
insertion attacks because they use this function.)

We therefore suggest that a string escaping function is included in a
future version of PostgreSQL and libpq. A sample implementation is
provided below, along with documentation.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Christopher Masto <chris(at)netmonger(dot)net>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-23 18:09:24
Message-ID: 20010823140924.B31597@netmonger.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Aug 22, 2001 at 05:16:44PM +0000, Florian Weimer wrote:
> We therefore suggest that a string escaping function is included in a
> future version of PostgreSQL and libpq. A sample implementation is
> provided below, along with documentation.

I use Perl, which (through DBD::Pg) has a "quote" function available,
but I think this is a very good idea to include in the library.

I only have one issue - the SQL standard seems to support the use
of '' to escape a single quote, but not \'. Though PostgreSQL has
an extended notion of character string literals, I think that the
usual policy of using the standard interface when possible should
apply.
--
Christopher Masto Senior Network Monkey NetMonger Communications
chris(at)netmonger(dot)net info(at)netmonger(dot)net http://www.netmonger.net

Free yourself, free your machine, free the daemon -- http://www.freebsd.org/


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-23 20:17:05
Message-ID: tgg0aio7jy.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Christopher Masto <chris(at)netmonger(dot)net> writes:

> I only have one issue - the SQL standard seems to support the use
> of '' to escape a single quote, but not \'. Though PostgreSQL has
> an extended notion of character string literals, I think that the
> usual policy of using the standard interface when possible should
> apply.

The first version escaped ' with ''. I changed it when I noticed that
if \' is used instead, the same function can be used for strings
('...') and identifiers ("...").

In addition, you have to replace \ with \\, so you are forced
to leave the grounds of the standard anyway.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-30 13:21:16
Message-ID: tgg0a9y983.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Florian Weimer <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de> writes:

> We therefore suggest that a string escaping function is included in a
> future version of PostgreSQL and libpq. A sample implementation is
> provided below, along with documentation.

We have now released a description of the problems which occur when a
string escaping function is not used:

http://cert.uni-stuttgart.de/advisories/apache_auth.php

What further steps are required to make the suggested patch part of
the official libpq library?

Thanks,
--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-30 22:43:25
Message-ID: 200108302243.f7UMhPr09909@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> It has come to our attention that many applications which use libpq
> are vulnerable to code insertion attacks in strings and identifiers
> passed to these applications. We have collected some evidence which
> suggests that this is related to the fact that libpq does not provide
> a function to escape strings and identifiers properly. (Both the
> Oracle and MySQL client libraries include such a function, and the
> vast majority of applications we examined are not vulnerable to code
> insertion attacks because they use this function.)
>
> We therefore suggest that a string escaping function is included in a
> future version of PostgreSQL and libpq. A sample implementation is
> provided below, along with documentation.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-30 22:43:55
Message-ID: 200108302243.f7UMhuP09937@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> Florian Weimer <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de> writes:
>
> > We therefore suggest that a string escaping function is included in a
> > future version of PostgreSQL and libpq. A sample implementation is
> > provided below, along with documentation.
>
> We have now released a description of the problems which occur when a
> string escaping function is not used:
>
> http://cert.uni-stuttgart.de/advisories/apache_auth.php
>
> What further steps are required to make the suggested patch part of
> the official libpq library?

Will be applied soon. I was waiting for comments before adding it to
the patch queue.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: "Mitch Vincent" <mvincent(at)cablespeed(dot)com>
To: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-30 23:07:36
Message-ID: 001501c131a8$8f567800$1e51000a@mitch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Perhaps I'm not thinking correctly but isn't it the job of the application
that's using the libpq library to escape special characters? I guess I don't
see a down side though, if it's implemented correctly to check and see if
characters are already escaped before escaping them (else major breakage of
existing application would occur).. I didn't see the patch but I assume that
someone took a look to make sure before applying it.

-Mitch

----- Original Message -----
From: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: "Florian Weimer" <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Sent: Thursday, August 30, 2001 6:43 PM
Subject: Re: [HACKERS] Escaping strings for inclusion into SQL queries

> > Florian Weimer <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de> writes:
> >
> > > We therefore suggest that a string escaping function is included in a
> > > future version of PostgreSQL and libpq. A sample implementation is
> > > provided below, along with documentation.
> >
> > We have now released a description of the problems which occur when a
> > string escaping function is not used:
> >
> > http://cert.uni-stuttgart.de/advisories/apache_auth.php
> >
> > What further steps are required to make the suggested patch part of
> > the official libpq library?
>
> Will be applied soon. I was waiting for comments before adding it to
> the patch queue.


From: Alex Pilosov <alex(at)pilosoft(dot)com>
To: Mitch Vincent <mvincent(at)cablespeed(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-30 23:32:58
Message-ID: Pine.BSO.4.10.10108301931440.19501-100000@spider.pilosoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

It is. Application is responsible to call PGescapeString (included in the
patch in question) to escape command that may possibly have user-specified
data... This function isn't called automatically.

On Thu, 30 Aug 2001, Mitch Vincent wrote:

> Perhaps I'm not thinking correctly but isn't it the job of the application
> that's using the libpq library to escape special characters? I guess I don't
> see a down side though, if it's implemented correctly to check and see if
> characters are already escaped before escaping them (else major breakage of
> existing application would occur).. I didn't see the patch but I assume that
> someone took a look to make sure before applying it.
>
>
> -Mitch
>
> ----- Original Message -----
> From: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>
> To: "Florian Weimer" <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de>
> Cc: <pgsql-hackers(at)postgresql(dot)org>
> Sent: Thursday, August 30, 2001 6:43 PM
> Subject: Re: [HACKERS] Escaping strings for inclusion into SQL queries
>
>
> > > Florian Weimer <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de> writes:
> > >
> > > > We therefore suggest that a string escaping function is included in a
> > > > future version of PostgreSQL and libpq. A sample implementation is
> > > > provided below, along with documentation.
> > >
> > > We have now released a description of the problems which occur when a
> > > string escaping function is not used:
> > >
> > > http://cert.uni-stuttgart.de/advisories/apache_auth.php
> > >
> > > What further steps are required to make the suggested patch part of
> > > the official libpq library?
> >
> > Will be applied soon. I was waiting for comments before adding it to
> > the patch queue.
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>
>


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-31 00:37:26
Message-ID: tgsne9uks9.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

"Mitch Vincent" <mvincent(at)cablespeed(dot)com> writes:

> Perhaps I'm not thinking correctly but isn't it the job of the application
> that's using the libpq library to escape special characters?

Yes, it is.

> I guess I don't see a down side though, if it's implemented
> correctly to check and see if characters are already escaped before
> escaping them (else major breakage of existing application would
> occur)..

You can't do this automatically because the strings needing escaping
are not marked in any way at the moment.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: "Mitch Vincent" <mvincent(at)cablespeed(dot)com>
To: "Alex Pilosov" <alex(at)pilosoft(dot)com>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-31 01:27:28
Message-ID: 002301c131bc$193c7610$be615dd8@mitch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Ok, I misudnerstood, I had long included my own escaping function in
programs that used libpq, I thought the intent was to make escaping happen
automatically..

Thanks!

-Mitch

----- Original Message -----
From: "Alex Pilosov" <alex(at)pilosoft(dot)com>
To: "Mitch Vincent" <mvincent(at)cablespeed(dot)com>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Sent: Thursday, August 30, 2001 7:32 PM
Subject: Re: [HACKERS] Escaping strings for inclusion into SQL queries

> It is. Application is responsible to call PGescapeString (included in the
> patch in question) to escape command that may possibly have user-specified
> data... This function isn't called automatically.
>
> On Thu, 30 Aug 2001, Mitch Vincent wrote:
>
> > Perhaps I'm not thinking correctly but isn't it the job of the
application
> > that's using the libpq library to escape special characters? I guess I
don't
> > see a down side though, if it's implemented correctly to check and see
if
> > characters are already escaped before escaping them (else major breakage
of
> > existing application would occur).. I didn't see the patch but I assume
that
> > someone took a look to make sure before applying it.
> >
> >
> > -Mitch
> >
> > ----- Original Message -----
> > From: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>
> > To: "Florian Weimer" <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de>
> > Cc: <pgsql-hackers(at)postgresql(dot)org>
> > Sent: Thursday, August 30, 2001 6:43 PM
> > Subject: Re: [HACKERS] Escaping strings for inclusion into SQL queries
> >
> >
> > > > Florian Weimer <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de> writes:
> > > >
> > > > > We therefore suggest that a string escaping function is included
in a
> > > > > future version of PostgreSQL and libpq. A sample implementation
is
> > > > > provided below, along with documentation.
> > > >
> > > > We have now released a description of the problems which occur when
a
> > > > string escaping function is not used:
> > > >
> > > > http://cert.uni-stuttgart.de/advisories/apache_auth.php
> > > >
> > > > What further steps are required to make the suggested patch part of
> > > > the official libpq library?
> > >
> > > Will be applied soon. I was waiting for comments before adding it to
> > > the patch queue.
> >
> >
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 6: Have you searched our list archives?
> >
> > http://www.postgresql.org/search.mpl
> >
> >
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>


From: Hannu Krosing <hannu(at)tm(dot)ee>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-31 03:52:35
Message-ID: 3B8F0A03.C68EEA2B@tm.ee
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian wrote:
>
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
> http://candle.pha.pa.us/cgi-bin/pgpatches
>
> I will try to apply it within the next 48 hours.
>
> > It has come to our attention that many applications which use libpq
> > are vulnerable to code insertion attacks in strings and identifiers
> > passed to these applications. We have collected some evidence which
> > suggests that this is related to the fact that libpq does not provide
> > a function to escape strings and identifiers properly. (Both the
> > Oracle and MySQL client libraries include such a function, and the
> > vast majority of applications we examined are not vulnerable to code
> > insertion attacks because they use this function.)

I think the real difference is what I complained in another mail to this
list -
in postgresql you can't do PREPARE / EXECUTE which could _automatically_
detect
where string escaping is needed or just eliminate the need for escaping.
In postgreSQL you have to construct all queries yourself by inserting
your
parameters inside your query strings in right places and escaping them
when
needed. That is unless you use an interface like ODBC/JDBS that fakes
the
PREPARE/EXECUTE on the client side and thus does the auto-escaping for
you .

I think that this should be added to TODO

* make portable BINARY representation for frontend-backend protocol by
using
typsend/typreceive functions for binary and typinput typoutput for
ASCII
(as currently typinput==typreceive and typoutput==typsend is suspect
the
usage to be inconsistent).

* make SQL changes to allow PREPARE/EXECUTE in main session, not only in
SPI

* make changes to client libraries to support marshalling arguments to
EXECUTE
using BINARY wire protocol or correctly escaped ASCII. The binary
protocol
would be very helpful for BYTEA and other big binary types.

> > We therefore suggest that a string escaping function is included in a
> > future version of PostgreSQL and libpq. A sample implementation is
> > provided below, along with documentation.

While you are at it you could also supply a standard query delimiter
function
as this is also a thing that seems to vary from db to db.

------------------
Hannu


From: Hannu Krosing <hannu(at)tm(dot)ee>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Barry Lind <barry(at)xythos(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-31 16:03:57
Message-ID: 3B8FB56D.74CAC792@tm.ee
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Barry Lind wrote:
>
> I agree with Hannu, that:
>
> * make SQL changes to allow PREPARE/EXECUTE in main session, not only
> in SPI

A more ambitious project would be

* develop an ANSI standard SQL/CLI compatible postgreSQL client library,
change wire protocol and SQL language as needed ;)

> is an important feature to expose out to the client. My primary reason
> is a perfomance one. Allowing the client to parse a SQL statement once
> and then supplying bind values for arguments and executing it multiple
> times can save a significant amount of server CPU, since the parsing and
> planning of the statement is only done once, even though multiple
> executions occur. This functionality is available in the backend
> (through SPI) and plpgsql uses it, but there isn't anyway to take
> advantage of this SPI functionality on the client (i.e. jdbc, odbc, etc.)
>
> I could see this implemented in different ways. One, by adding new SQL
> commands to bind or execute an already open statement, or two, by
> changing the FE/BE protocol to allow the client to open, parse,
> describe, bind, execute and close a statement as separate actions that
> can be sent to the server in one or more requests. (The latter is how
> Oracle does it).

The latter is also the ODBS and JDBC wiew of how it is done. The current
PG drivers have to fake it all on client side.

>
> I also would like to see this added to the todo list.
>

------------
Hannu


From: Barry Lind <barry(at)xythos(dot)com>
To: Hannu Krosing <hannu(at)tm(dot)ee>
Cc: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-08-31 17:03:04
Message-ID: 3B8FC348.5020501@xythos.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I agree with Hannu, that:

* make SQL changes to allow PREPARE/EXECUTE in main session, not only
in SPI

is an important feature to expose out to the client. My primary reason
is a perfomance one. Allowing the client to parse a SQL statement once
and then supplying bind values for arguments and executing it multiple
times can save a significant amount of server CPU, since the parsing and
planning of the statement is only done once, even though multiple
executions occur. This functionality is available in the backend
(through SPI) and plpgsql uses it, but there isn't anyway to take
advantage of this SPI functionality on the client (i.e. jdbc, odbc, etc.)

I could see this implemented in different ways. One, by adding new SQL
commands to bind or execute an already open statement, or two, by
changing the FE/BE protocol to allow the client to open, parse,
describe, bind, execute and close a statement as separate actions that
can be sent to the server in one or more requests. (The latter is how
Oracle does it).

I also would like to see this added to the todo list.

thanks,
--Barry

Hannu Krosing wrote:
> Bruce Momjian wrote:
>
>>Your patch has been added to the PostgreSQL unapplied patches list at:
>>
>> http://candle.pha.pa.us/cgi-bin/pgpatches
>>
>>I will try to apply it within the next 48 hours.
>>
>>
>>>It has come to our attention that many applications which use libpq
>>>are vulnerable to code insertion attacks in strings and identifiers
>>>passed to these applications. We have collected some evidence which
>>>suggests that this is related to the fact that libpq does not provide
>>>a function to escape strings and identifiers properly. (Both the
>>>Oracle and MySQL client libraries include such a function, and the
>>>vast majority of applications we examined are not vulnerable to code
>>>insertion attacks because they use this function.)
>>>
>
> I think the real difference is what I complained in another mail to this
> list -
> in postgresql you can't do PREPARE / EXECUTE which could _automatically_
> detect
> where string escaping is needed or just eliminate the need for escaping.
> In postgreSQL you have to construct all queries yourself by inserting
> your
> parameters inside your query strings in right places and escaping them
> when
> needed. That is unless you use an interface like ODBC/JDBS that fakes
> the
> PREPARE/EXECUTE on the client side and thus does the auto-escaping for
> you .
>
>
> I think that this should be added to TODO
>
> * make portable BINARY representation for frontend-backend protocol by
> using
> typsend/typreceive functions for binary and typinput typoutput for
> ASCII
> (as currently typinput==typreceive and typoutput==typsend is suspect
> the
> usage to be inconsistent).
>
> * make SQL changes to allow PREPARE/EXECUTE in main session, not only in
> SPI
>
> * make changes to client libraries to support marshalling arguments to
> EXECUTE
> using BINARY wire protocol or correctly escaped ASCII. The binary
> protocol
> would be very helpful for BYTEA and other big binary types.
>
>
>
>>>We therefore suggest that a string escaping function is included in a
>>>future version of PostgreSQL and libpq. A sample implementation is
>>>provided below, along with documentation.
>>>
>
> While you are at it you could also supply a standard query delimiter
> function
> as this is also a thing that seems to vary from db to db.
>
> ------------------
> Hannu
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>
>


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-01 07:53:48
Message-ID: Pine.LNX.4.30.0109010953050.722-100000@peter.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

For consistency with the rest of the libpq API, the function should be
called PQescapeString, not PGescapeString.

Bruce Momjian writes:

>
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
> http://candle.pha.pa.us/cgi-bin/pgpatches
>
> I will try to apply it within the next 48 hours.
>
> > It has come to our attention that many applications which use libpq
> > are vulnerable to code insertion attacks in strings and identifiers
> > passed to these applications. We have collected some evidence which
> > suggests that this is related to the fact that libpq does not provide
> > a function to escape strings and identifiers properly. (Both the
> > Oracle and MySQL client libraries include such a function, and the
> > vast majority of applications we examined are not vulnerable to code
> > insertion attacks because they use this function.)
> >
> > We therefore suggest that a string escaping function is included in a
> > future version of PostgreSQL and libpq. A sample implementation is
> > provided below, along with documentation.
> >
> > --
> > Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> > University of Stuttgart http://cert.uni-stuttgart.de/
> > RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 2: you can get off all lists at once with the unregister command
> > (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>
>

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-01 07:57:00
Message-ID: Pine.LNX.4.30.0109010954520.722-100000@peter.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Florian Weimer writes:

> The first version escaped ' with ''. I changed it when I noticed that
> if \' is used instead, the same function can be used for strings
> ('...') and identifiers ("...").

Last time I checked (15 seconds ago), you could not escape " with \ in
PostgreSQL. The identifer parsing rules are a bit different from strings.

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-03 16:03:37
Message-ID: tgheukl0rq.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:

> Florian Weimer writes:
>
> > The first version escaped ' with ''. I changed it when I noticed that
> > if \' is used instead, the same function can be used for strings
> > ('...') and identifiers ("...").
>
> Last time I checked (15 seconds ago), you could not escape " with \ in
> PostgreSQL. The identifer parsing rules are a bit different from strings.

Yes, we misread the lexer description. I'm sorry about that.

In addition, there seems to be a bug in the treatment of "" escapes in
identifiers. 'SELECT """";' yields the error message 'Attribute '""'
not found ' (not '"'!) or even 'Attribute '""\' not found', depending
on the queries executed before.

For identifiers, comparing the characters to a white list is probably
a more reasonable approach.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-03 20:28:49
Message-ID: 200109032028.f83KSnD18708@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


OK, can you supply an updated patch?

> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>
> > Florian Weimer writes:
> >
> > > The first version escaped ' with ''. I changed it when I noticed that
> > > if \' is used instead, the same function can be used for strings
> > > ('...') and identifiers ("...").
> >
> > Last time I checked (15 seconds ago), you could not escape " with \ in
> > PostgreSQL. The identifer parsing rules are a bit different from strings.
>
> Yes, we misread the lexer description. I'm sorry about that.
>
> In addition, there seems to be a bug in the treatment of "" escapes in
> identifiers. 'SELECT """";' yields the error message 'Attribute '""'
> not found ' (not '"'!) or even 'Attribute '""\' not found', depending
> on the queries executed before.
>
> For identifiers, comparing the characters to a white list is probably
> a more reasonable approach.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-03 20:59:54
Message-ID: tgd758f0s5.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:

> OK, can you supply an updated patch?

Yes, I'm going to update it. Shall I post it here?

Could anybody have a look at the parser issue?

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-03 21:19:44
Message-ID: 200109032119.f83LJit22899@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
> > OK, can you supply an updated patch?
>
> Yes, I'm going to update it. Shall I post it here?

Sure, or patches list.

> Could anybody have a look at the parser issue?

I am unsure how it is supposed to behave. Comments? Does the standard
say anything?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-03 22:17:51
Message-ID: Pine.LNX.4.30.0109040016270.4304-100000@peter.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Florian Weimer writes:

> In addition, there seems to be a bug in the treatment of "" escapes in
> identifiers. 'SELECT """";' yields the error message 'Attribute '""'
> not found ' (not '"'!) or even 'Attribute '""\' not found', depending
> on the queries executed before.

A bug indeed.

RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/scan.l,v
retrieving revision 1.88
diff -u -r1.88 scan.l
--- scan.l 2001/03/22 17:41:47 1.88
+++ scan.l 2001/09/03 22:11:46
@@ -375,7 +375,7 @@
return IDENT;
}
<xd>{xddouble} {
- addlit(yytext, yyleng-1);
+ addlit(yytext+1, yyleng-1);
}
<xd>{xdinside} {
addlit(yytext, yyleng);
===end

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 00:10:19
Message-ID: 12273.999562219@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> A bug indeed.

> <xd>{xddouble} {
> - addlit(yytext, yyleng-1);
> + addlit(yytext+1, yyleng-1);
> }

I don't follow. xddouble can only expand to two quote marks, so how
does it matter which one we use as the result? This seems unlikely
to change the behavior. If it does, I think the real bug is elsewhere.

I do see a bug here --- I get

regression=# select """";
NOTICE: identifier """ [ lots o' rubouts ] @;" will be truncated to """"
ERROR: Attribute '""' not found
regression=#

regards, tom lane


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 00:40:38
Message-ID: Pine.LNX.4.30.0109040231390.4304-100000@peter.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane writes:

> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > A bug indeed.
>
> > <xd>{xddouble} {
> > - addlit(yytext, yyleng-1);
> > + addlit(yytext+1, yyleng-1);
> > }
>
> I don't follow. xddouble can only expand to two quote marks, so how
> does it matter which one we use as the result?

addlit() expects the first argument to be null-terminated and implicitly
uses that null byte at the end of the supplied argument to terminate its
own buffer. It expects to copy <doublequote><null> (new version), whereas
it got (old version) <doublequote><doublequote> and left the buffer
unterminated, which leads to random behavior, as you saw.

Since there are only a few calls to addlit(), I didn't feel like
re-engineering the whole interface to be prettier. It does look like a
performance-beneficial implementation.

A concern related to the matter is that if you actually put such an
identifier into your database you basically make it undumpable (well,
unrestorable) because no place is prepared to handle such a thing.

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 00:44:36
Message-ID: 15611.999564276@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Tom Lane writes:
>> I don't follow. xddouble can only expand to two quote marks, so how
>> does it matter which one we use as the result?

> addlit() expects the first argument to be null-terminated and implicitly
> uses that null byte at the end of the supplied argument to terminate its
> own buffer.

Hmm, so I see:

/* append data --- note we assume ytext is null-terminated */
memcpy(literalbuf+literallen, ytext, yleng+1);
literallen += yleng;

Given that we are passing the length of the desired string, it seems
bug-prone for addlit to *also* expect null termination. I'd suggest

memcpy(literalbuf+literallen, ytext, yleng);
literallen += yleng;
literalbuf[literallen] = '\0';

instead.

regards, tom lane


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 00:59:41
Message-ID: 200109040059.f840xf806810@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I am going to apply this patch with the change that the function name
will be PQ* not PG*.

> It has come to our attention that many applications which use libpq
> are vulnerable to code insertion attacks in strings and identifiers
> passed to these applications. We have collected some evidence which
> suggests that this is related to the fact that libpq does not provide
> a function to escape strings and identifiers properly. (Both the
> Oracle and MySQL client libraries include such a function, and the
> vast majority of applications we examined are not vulnerable to code
> insertion attacks because they use this function.)
>
> We therefore suggest that a string escaping function is included in a
> future version of PostgreSQL and libpq. A sample implementation is
> provided below, along with documentation.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 17:30:54
Message-ID: 200109041730.f84HUsI19449@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Patch removed at the request of the author. Author will resubmit.

> It has come to our attention that many applications which use libpq
> are vulnerable to code insertion attacks in strings and identifiers
> passed to these applications. We have collected some evidence which
> suggests that this is related to the fact that libpq does not provide
> a function to escape strings and identifiers properly. (Both the
> Oracle and MySQL client libraries include such a function, and the
> vast majority of applications we examined are not vulnerable to code
> insertion attacks because they use this function.)
>
> We therefore suggest that a string escaping function is included in a
> future version of PostgreSQL and libpq. A sample implementation is
> provided below, along with documentation.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-04 18:42:47
Message-ID: tg66ay94rc.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:

> Patch removed at the request of the author. Author will resubmit.

I've attached the fixed version of the patch below. After the
discussion on pgsql-hackers (especially the frightening memory dump in
<12273(dot)999562219(at)sss(dot)pgh(dot)pa(dot)us>), we decided that it is best not to
use identifiers from an untrusted source at all. Therefore, all
claims of the suitability of PQescapeString() for identifiers have
been removed.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-07 20:16:48
Message-ID: 200109072016.f87KGmh21495@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Has this been resolved?

> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > Tom Lane writes:
> >> I don't follow. xddouble can only expand to two quote marks, so how
> >> does it matter which one we use as the result?
>
> > addlit() expects the first argument to be null-terminated and implicitly
> > uses that null byte at the end of the supplied argument to terminate its
> > own buffer.
>
> Hmm, so I see:
>
> /* append data --- note we assume ytext is null-terminated */
> memcpy(literalbuf+literallen, ytext, yleng+1);
> literallen += yleng;
>
> Given that we are passing the length of the desired string, it seems
> bug-prone for addlit to *also* expect null termination. I'd suggest
>
> memcpy(literalbuf+literallen, ytext, yleng);
> literallen += yleng;
> literalbuf[literallen] = '\0';
>
> instead.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-07 20:26:50
Message-ID: 4368.999894410@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Has this been resolved?

Peter applied his patch, but I am planning to also change addlit to not
require null termination, because I believe we'll get bit again if we
don't.

regards, tom lane


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-07 21:26:05
Message-ID: 200109072126.f87LQ6R28944@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
> > Patch removed at the request of the author. Author will resubmit.
>
> I've attached the fixed version of the patch below. After the
> discussion on pgsql-hackers (especially the frightening memory dump in
> <12273(dot)999562219(at)sss(dot)pgh(dot)pa(dot)us>), we decided that it is best not to
> use identifiers from an untrusted source at all. Therefore, all
> claims of the suitability of PQescapeString() for identifiers have
> been removed.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>

> Index: doc/src/sgml/libpq.sgml
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
> retrieving revision 1.68
> diff -u -r1.68 libpq.sgml
> --- doc/src/sgml/libpq.sgml 2001/09/04 00:18:18 1.68
> +++ doc/src/sgml/libpq.sgml 2001/09/04 18:32:05
> @@ -827,6 +827,42 @@
> </itemizedlist>
> </sect2>
>
> +<sect2 id="libpq-exec-escape-string">
> + <title>Escaping strings for inclusion in SQL queries</title>
> +<para>
> +<function>PQescapeString</function>
> + Escapes a string for use within an SQL query.
> +<synopsis>
> +size_t PQescapeString (char *to, const char *from, size_t length);
> +</synopsis>
> +If you want to include strings which have been received
> +from a source which is not trustworthy (for example, because they were
> +transmitted across a network), you cannot directly include them in SQL
> +queries for security reasons. Instead, you have to quote special
> +characters which are otherwise interpreted by the SQL parser.
> +</para>
> +<para>
> +<function>PQescapeString</> performs this operation. The
> +<parameter>from</> points to the first character of the string which
> +is to be escaped, and the <parameter>length</> parameter counts the
> +number of characters in this string (a terminating NUL character is
> +neither necessary nor counted). <parameter>to</> shall point to a
> +buffer which is able to hold at least one more character than twice
> +the value of <parameter>length</>, otherwise the behavior is
> +undefined. A call to <function>PQescapeString</> writes an escaped
> +version of the <parameter>from</> string to the <parameter>to</>
> +buffer, replacing special characters so that they cannot cause any
> +harm, and adding a terminating NUL character. The single quotes which
> +must surround PostgreSQL string literals are not part of the result
> +string.
> +</para>
> +<para>
> +<function>PQescapeString</> returns the number of characters written
> +to <parameter>to</>, not including the terminating NUL character.
> +Behavior is undefined when the <parameter>to</> and <parameter>from</>
> +strings overlap.
> +</para>
> +
> <sect2 id="libpq-exec-select-info">
> <title>Retrieving SELECT Result Information</title>
>
> Index: src/interfaces/libpq/fe-exec.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
> retrieving revision 1.108
> diff -u -r1.108 fe-exec.c
> --- src/interfaces/libpq/fe-exec.c 2001/08/21 20:39:53 1.108
> +++ src/interfaces/libpq/fe-exec.c 2001/09/04 18:32:09
> @@ -56,6 +56,62 @@
> static int getNotify(PGconn *conn);
> static int getNotice(PGconn *conn);
>
> +/* ---------------
> + * Escaping arbitrary strings to get valid SQL strings/identifiers.
> + *
> + * Replaces "\\" with "\\\\", "\0" with "\\0", and "'" with "''".
> + * length is the length of the buffer pointed to by
> + * from. The buffer at to must be at least 2*length + 1 characters
> + * long. A terminating NUL character is written.
> + * ---------------
> + */
> +
> +size_t
> +PQescapeString (char *to, const char *from, size_t length)
> +{
> + const char *source = from;
> + char *target = to;
> + unsigned int remaining = length;
> +
> + while (remaining > 0) {
> + switch (*source) {
> + case '\0':
> + *target = '\\';
> + target++;
> + *target = '0';
> + /* target and remaining are updated below. */
> + break;
> +
> + case '\\':
> + *target = '\\';
> + target++;
> + *target = '\\';
> + /* target and remaining are updated below. */
> + break;
> +
> + case '\'':
> + *target = '\'';
> + target++;
> + *target = '\'';
> + /* target and remaining are updated below. */
> + break;
> +
> + default:
> + *target = *source;
> + /* target and remaining are updated below. */
> + }
> + source++;
> + target++;
> + remaining--;
> + }
> +
> + /* Write the terminating NUL character. */
> + *target = '\0';
> +
> + return target - to;
> +}
> +
> +
>
> /* ----------------
> * Space management for PGresult.
> Index: src/interfaces/libpq/libpq-fe.h
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
> retrieving revision 1.72
> diff -u -r1.72 libpq-fe.h
> --- src/interfaces/libpq/libpq-fe.h 2001/08/21 20:39:54 1.72
> +++ src/interfaces/libpq/libpq-fe.h 2001/09/04 18:32:09
> @@ -251,6 +251,9 @@
>
> /* === in fe-exec.c === */
>
> + /* Quoting strings before inclusion in queries. */
> + extern size_t PQescapeString (char *to, const char *from, size_t length);
> +
> /* Simple synchronous query */
> extern PGresult *PQexec(PGconn *conn, const char *query);
> extern PGnotify *PQnotifies(PGconn *conn);
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-07 22:02:32
Message-ID: 200109072202.f87M2Wb00980@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Patch applied. Thanks.

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
> > Patch removed at the request of the author. Author will resubmit.
>
> I've attached the fixed version of the patch below. After the
> discussion on pgsql-hackers (especially the frightening memory dump in
> <12273(dot)999562219(at)sss(dot)pgh(dot)pa(dot)us>), we decided that it is best not to
> use identifiers from an untrusted source at all. Therefore, all
> claims of the suitability of PQescapeString() for identifiers have
> been removed.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>

> Index: doc/src/sgml/libpq.sgml
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
> retrieving revision 1.68
> diff -u -r1.68 libpq.sgml
> --- doc/src/sgml/libpq.sgml 2001/09/04 00:18:18 1.68
> +++ doc/src/sgml/libpq.sgml 2001/09/04 18:32:05
> @@ -827,6 +827,42 @@
> </itemizedlist>
> </sect2>
>
> +<sect2 id="libpq-exec-escape-string">
> + <title>Escaping strings for inclusion in SQL queries</title>
> +<para>
> +<function>PQescapeString</function>
> + Escapes a string for use within an SQL query.
> +<synopsis>
> +size_t PQescapeString (char *to, const char *from, size_t length);
> +</synopsis>
> +If you want to include strings which have been received
> +from a source which is not trustworthy (for example, because they were
> +transmitted across a network), you cannot directly include them in SQL
> +queries for security reasons. Instead, you have to quote special
> +characters which are otherwise interpreted by the SQL parser.
> +</para>
> +<para>
> +<function>PQescapeString</> performs this operation. The
> +<parameter>from</> points to the first character of the string which
> +is to be escaped, and the <parameter>length</> parameter counts the
> +number of characters in this string (a terminating NUL character is
> +neither necessary nor counted). <parameter>to</> shall point to a
> +buffer which is able to hold at least one more character than twice
> +the value of <parameter>length</>, otherwise the behavior is
> +undefined. A call to <function>PQescapeString</> writes an escaped
> +version of the <parameter>from</> string to the <parameter>to</>
> +buffer, replacing special characters so that they cannot cause any
> +harm, and adding a terminating NUL character. The single quotes which
> +must surround PostgreSQL string literals are not part of the result
> +string.
> +</para>
> +<para>
> +<function>PQescapeString</> returns the number of characters written
> +to <parameter>to</>, not including the terminating NUL character.
> +Behavior is undefined when the <parameter>to</> and <parameter>from</>
> +strings overlap.
> +</para>
> +
> <sect2 id="libpq-exec-select-info">
> <title>Retrieving SELECT Result Information</title>
>
> Index: src/interfaces/libpq/fe-exec.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
> retrieving revision 1.108
> diff -u -r1.108 fe-exec.c
> --- src/interfaces/libpq/fe-exec.c 2001/08/21 20:39:53 1.108
> +++ src/interfaces/libpq/fe-exec.c 2001/09/04 18:32:09
> @@ -56,6 +56,62 @@
> static int getNotify(PGconn *conn);
> static int getNotice(PGconn *conn);
>
> +/* ---------------
> + * Escaping arbitrary strings to get valid SQL strings/identifiers.
> + *
> + * Replaces "\\" with "\\\\", "\0" with "\\0", and "'" with "''".
> + * length is the length of the buffer pointed to by
> + * from. The buffer at to must be at least 2*length + 1 characters
> + * long. A terminating NUL character is written.
> + * ---------------
> + */
> +
> +size_t
> +PQescapeString (char *to, const char *from, size_t length)
> +{
> + const char *source = from;
> + char *target = to;
> + unsigned int remaining = length;
> +
> + while (remaining > 0) {
> + switch (*source) {
> + case '\0':
> + *target = '\\';
> + target++;
> + *target = '0';
> + /* target and remaining are updated below. */
> + break;
> +
> + case '\\':
> + *target = '\\';
> + target++;
> + *target = '\\';
> + /* target and remaining are updated below. */
> + break;
> +
> + case '\'':
> + *target = '\'';
> + target++;
> + *target = '\'';
> + /* target and remaining are updated below. */
> + break;
> +
> + default:
> + *target = *source;
> + /* target and remaining are updated below. */
> + }
> + source++;
> + target++;
> + remaining--;
> + }
> +
> + /* Write the terminating NUL character. */
> + *target = '\0';
> +
> + return target - to;
> +}
> +
> +
>
> /* ----------------
> * Space management for PGresult.
> Index: src/interfaces/libpq/libpq-fe.h
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
> retrieving revision 1.72
> diff -u -r1.72 libpq-fe.h
> --- src/interfaces/libpq/libpq-fe.h 2001/08/21 20:39:54 1.72
> +++ src/interfaces/libpq/libpq-fe.h 2001/09/04 18:32:09
> @@ -251,6 +251,9 @@
>
> /* === in fe-exec.c === */
>
> + /* Quoting strings before inclusion in queries. */
> + extern size_t PQescapeString (char *to, const char *from, size_t length);
> +
> /* Simple synchronous query */
> extern PGresult *PQexec(PGconn *conn, const char *query);
> extern PGnotify *PQnotifies(PGconn *conn);
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: "Joe Conway" <joseph(dot)conway(at)home(dot)com>
To: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>, "Florian Weimer" <Florian(dot)Weimer(at)rus(dot)uni-stuttgart(dot)de>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-11 06:26:23
Message-ID: 01c701c13a8a$adf7f190$0705a8c0@jecw2k1
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> Patch applied. Thanks.
>
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >
> > > Patch removed at the request of the author. Author will resubmit.
> >
> > I've attached the fixed version of the patch below. After the
> > discussion on pgsql-hackers (especially the frightening memory dump in
> > <12273(dot)999562219(at)sss(dot)pgh(dot)pa(dot)us>), we decided that it is best not to
> > use identifiers from an untrusted source at all. Therefore, all
> > claims of the suitability of PQescapeString() for identifiers have
> > been removed.

I found a problem with PQescapeString (I think). Since it escapes
null bytes to be literally '\0', the following can happen:
1. User inputs string value as "<null byte>##" where ## are digits in the
range of 0 to 7.
2. PQescapeString converts this to "\0##"
3. Escaped string is used in a context that causes "\0##" to be evaluated as
an octal escape sequence.

For example, if the user enters a null byte followed by "47", and escapes
it, it becomes "\071" which gets translated into a single digit "9" by the
general parser. Along the same lines, if there is a null byte in a string,
and it is not followed by digits, the resulting "\0" gets converted back
into a null byte by the parser and the string gets truncated.

If the goal is to "safely" encode null bytes, and preserve the rest of the
string as it was entered, I think the null bytes should be escaped as \\000
(note that if you simply use \000 the same string truncation problem
occurs).

-- Joe


From: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
To: "Joe Conway" <joseph(dot)conway(at)home(dot)com>
Cc: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-11 14:22:55
Message-ID: tg3d5tajsw.fsf@mercury.rus.uni-stuttgart.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

"Joe Conway" <joseph(dot)conway(at)home(dot)com> writes:

> I found a problem with PQescapeString (I think). Since it escapes
> null bytes to be literally '\0', the following can happen:
> 1. User inputs string value as "<null byte>##" where ## are digits in the
> range of 0 to 7.
> 2. PQescapeString converts this to "\0##"
> 3. Escaped string is used in a context that causes "\0##" to be evaluated as
> an octal escape sequence.

I agree that this is a problem, though it is not possible to do
anything harmful with it. In addition, it only occurs if there are
any NUL characters in its input, which is very unlikely if you are
using C strings.

The patch below addresses the issue by removing escaping of \0
characters entirely.

> If the goal is to "safely" encode null bytes, and preserve the rest of the
> string as it was entered, I think the null bytes should be escaped as \\000
> (note that if you simply use \000 the same string truncation problem
> occurs).

We can't do that, this would require 4n + 1 bytes of storage for the
result, breaking the interface.

--
Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: Joe Conway <joseph(dot)conway(at)home(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queries
Date: 2001-09-12 04:20:31
Message-ID: 200109120420.f8C4KVf28273@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


I think we need this patch. Bytea encoding will be changed to accept
\000 rather than \0 for the same reason. I also agree that the libpq
enescaping of a C string doesn't need to deal with NULL like bytea does.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> "Joe Conway" <joseph(dot)conway(at)home(dot)com> writes:
>
> > I found a problem with PQescapeString (I think). Since it escapes
> > null bytes to be literally '\0', the following can happen:
> > 1. User inputs string value as "<null byte>##" where ## are digits in the
> > range of 0 to 7.
> > 2. PQescapeString converts this to "\0##"
> > 3. Escaped string is used in a context that causes "\0##" to be evaluated as
> > an octal escape sequence.
>
> I agree that this is a problem, though it is not possible to do
> anything harmful with it. In addition, it only occurs if there are
> any NUL characters in its input, which is very unlikely if you are
> using C strings.
>
> The patch below addresses the issue by removing escaping of \0
> characters entirely.
>
> > If the goal is to "safely" encode null bytes, and preserve the rest of the
> > string as it was entered, I think the null bytes should be escaped as \\000
> > (note that if you simply use \000 the same string truncation problem
> > occurs).
>
> We can't do that, this would require 4n + 1 bytes of storage for the
> result, breaking the interface.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Florian Weimer <Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE>
Cc: Joe Conway <joseph(dot)conway(at)home(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Escaping strings for inclusion into SQL queriesh
Date: 2001-09-13 17:00:26
Message-ID: 200109131700.f8DH0Qk26700@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Patch applied. Thanks.

> "Joe Conway" <joseph(dot)conway(at)home(dot)com> writes:
>
> > I found a problem with PQescapeString (I think). Since it escapes
> > null bytes to be literally '\0', the following can happen:
> > 1. User inputs string value as "<null byte>##" where ## are digits in the
> > range of 0 to 7.
> > 2. PQescapeString converts this to "\0##"
> > 3. Escaped string is used in a context that causes "\0##" to be evaluated as
> > an octal escape sequence.
>
> I agree that this is a problem, though it is not possible to do
> anything harmful with it. In addition, it only occurs if there are
> any NUL characters in its input, which is very unlikely if you are
> using C strings.
>
> The patch below addresses the issue by removing escaping of \0
> characters entirely.
>
> > If the goal is to "safely" encode null bytes, and preserve the rest of the
> > string as it was entered, I think the null bytes should be escaped as \\000
> > (note that if you simply use \000 the same string truncation problem
> > occurs).
>
> We can't do that, this would require 4n + 1 bytes of storage for the
> result, breaking the interface.
>
> --
> Florian Weimer Florian(dot)Weimer(at)RUS(dot)Uni-Stuttgart(dot)DE
> University of Stuttgart http://cert.uni-stuttgart.de/
> RUS-CERT +49-711-685-5973/fax +49-711-685-5898
>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026