Thread: Escaping strings for inclusion into SQL queries
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
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, Pennsylvania19026
> 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, Pennsylvania19026
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 > > 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.
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 > > > 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 > >
"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 > > > > 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 >
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
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 > >
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
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
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
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,
Pennsylvania19026
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, Pennsylvania19026
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, Pennsylvania19026
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, Pennsylvania19026
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);
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, Pennsylvania19026
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, Pennsylvania19026
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, Pennsylvania19026
> 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
Attachment
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, Pennsylvania19026
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, Pennsylvania19026