Escaping strings for inclusion into SQL queries

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@netmonger.net info@netmonger.net http://www.netmonger.net

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

Christopher Masto <chris@netmonger.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

Florian Weimer <Florian.Weimer@rus.uni-stuttgart.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org)

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

Florian Weimer <Florian.Weimer@rus.uni-stuttgart.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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

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@candle.pha.pa.us>
To: "Florian Weimer" <Florian.Weimer@rus.uni-stuttgart.de>
Cc: <pgsql-hackers@postgresql.org>
Sent: Thursday, August 30, 2001 6:43 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:

"Mitch Vincent" <mvincent@cablespeed.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

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@pilosoft.com>
To: "Mitch Vincent" <mvincent@cablespeed.com>
Cc: <pgsql-hackers@postgresql.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@candle.pha.pa.us>
To: "Florian Weimer" <Florian.Weimer@rus.uni-stuttgart.de>
Cc: <pgsql-hackers@postgresql.org>
Sent: Thursday, August 30, 2001 6:43 PM
Subject: Re: [HACKERS] Escaping strings for inclusion into SQL queries

Florian Weimer <Florian.Weimer@rus.uni-stuttgart.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

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

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

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:

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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org)

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

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@gmx.net http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

OK, can you supply an updated patch?

Peter Eisentraut <peter_e@gmx.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.Weimer@RUS.Uni-Stuttgart.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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

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@gmx.net http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.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

Tom Lane writes:

Peter Eisentraut <peter_e@gmx.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@gmx.net http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.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

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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org)

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

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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org)

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

Bruce Momjian <pgman@candle.pha.pa.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.999562219@sss.pgh.pa.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

Has this been resolved?

Peter Eisentraut <peter_e@gmx.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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.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

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@candle.pha.pa.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.999562219@sss.pgh.pa.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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org

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

Patch applied. Thanks.

Bruce Momjian <pgman@candle.pha.pa.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.999562219@sss.pgh.pa.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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org

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

Patch applied. Thanks.

Bruce Momjian <pgman@candle.pha.pa.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.999562219@sss.pgh.pa.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

"Joe Conway" <joseph.conway@home.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.Weimer@RUS.Uni-Stuttgart.DE
University of Stuttgart http://cert.uni-stuttgart.de/
RUS-CERT +49-711-685-5973/fax +49-711-685-5898

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.conway@home.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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org so that your
message can get through to the mailing list cleanly

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

Patch applied. Thanks.

"Joe Conway" <joseph.conway@home.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.Weimer@RUS.Uni-Stuttgart.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@postgresql.org so that your
message can get through to the mailing list cleanly

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