possible design bug with PQescapeString()

Started by Tatsuo Ishiialmost 20 years ago20 messages
#1Tatsuo Ishii
ishii@sraoss.co.jp

I believe PQescapeString() has an important design bug and it casues a
security risk.

The function's signature is:

size_t PQescapeString (char *to, const char *from, size_t length);

As you might notice, it's impossible to specify encoding of "to". As a
result, it turns every occurrences of 0x27(') or 0x5c(\) to 0x270x27
or 0x5c0x5c. This is fine with ASCII, UTF-8, EUC-JP and so on. However
cetain Asian multibyte charsets such as SJIS, Big5 and GBK have a bit
pattern in that the second byte is 0x27(') or 0x5c(\). Applying
PQescapeString() to them will produce invalid character sequences.

But there's more. Problem is, PQescapeString() makes SQL injections
possible. Here is an example:

There is an application which selects particlular member info from a
table in this way:

SELECT * FROM members WHERE member_name = 'var';

Users can input value for "var" from a web form. The attacker inputs
following string:

(0x95+0x27);DELETE FROM members;--

where 0x95+0x27 is actually a SJIS mutibyte KANJI. Programmer applies
PQescapeString() to it and gets:

0x95+0x27+0x27;DELETE FROM members;--

and the result SQL will be:

SELECT * FROM members WHERE member_name = '0x95+0x27';DELETE FROM members;--';

You lose members table:-<

Conclusion:

I suggest that PQescapeString() should have a parameter to specify the
encoding of "to".

BTW it's irony that PQescapeStringt man page stats like this:-)

Tip: It is especially important to do proper escaping when handling
strings that were received from an untrustworthy source. Otherwise
there is a security risk: you are vulnerable to "SQL injection"
attacks wherein unwanted SQL commands are fed to your database.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuo Ishii (#1)
Re: possible design bug with PQescapeString()

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I suggest that PQescapeString() should have a parameter to specify the
encoding of "to".

You mean the encoding of "from", no? But actually I'd argue that
letting the client programmer supply the encoding is still a pretty
dangerous practice. Your example demonstrates that if the encoding
PQescapeString is told is different from the encoding the backend parser
thinks is in use, problems result. Perhaps we should pass the PGconn
to new-PQescapeString and let it dig the client encoding out of that.

You could still get burnt if the client encoding changes between the
invocation of new-PQescapeString and the sending of the constructed
command, but that's a fairly unlikely case.

The bottom line to this though is that these encodings are just plain
dangerous. I'm more than half tempted to suggest that the only secure
answer is to drop support for these encodings. Consider for example
an application that isn't using PQescapeString but has its own
double-backslashes-and-quotes logic embedded. You can break it if you
can manage to get the backend to think that the client encoding is SJIS
or similar. That's a hazard we're basically not ever going to be able
to prevent.

regards, tom lane

#3Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Tom Lane (#2)
Re: possible design bug with PQescapeString()

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I suggest that PQescapeString() should have a parameter to specify the
encoding of "to".

You mean the encoding of "from", no?

Oops, "from", yes.

But actually I'd argue that
letting the client programmer supply the encoding is still a pretty
dangerous practice. Your example demonstrates that if the encoding
PQescapeString is told is different from the encoding the backend parser
thinks is in use, problems result. Perhaps we should pass the PGconn
to new-PQescapeString and let it dig the client encoding out of that.

Sound good to pass PGconn to new-PQescapeString. Here is the proposed
calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char *to, const char *from, size_t length)

If this is ok, I will implement for 8.2.

You could still get burnt if the client encoding changes between the
invocation of new-PQescapeString and the sending of the constructed
command, but that's a fairly unlikely case.

The bottom line to this though is that these encodings are just plain
dangerous. I'm more than half tempted to suggest that the only secure
answer is to drop support for these encodings. Consider for example
an application that isn't using PQescapeString but has its own
double-backslashes-and-quotes logic embedded. You can break it if you
can manage to get the backend to think that the client encoding is SJIS
or similar. That's a hazard we're basically not ever going to be able
to prevent.

Dropping support for SJIS and so on will not be practical at all since
these encodings has been widely used and I don't see these encodings
are deprecated in the near future. I think dropping the support will
simply prevent people from using PostgreSQL. Especially in Windows
world, these encodings are pretty common.

I know that these encodings are broken in their design and actually I
hate them:-) But this is real world and we have to live with them...
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#4Florian Weimer
fw@deneb.enyo.de
In reply to: Tatsuo Ishii (#1)
Re: possible design bug with PQescapeString()

* Tatsuo Ishii:

Users can input value for "var" from a web form. The attacker inputs
following string:

(0x95+0x27);DELETE FROM members;--

where 0x95+0x27 is actually a SJIS mutibyte KANJI. Programmer applies
PQescapeString() to it and gets:

0x95+0x27+0x27;DELETE FROM members;--

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126. Unfortunately, there is nothing we can do about
this in the current function because tha twould need four times the
lenggth of the input string (plus one). Drat.

(I don't think you should have to consider the encoding in the client;
strange things may happen if there is an interpretation conflict
between the client and the backend.)

#5Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Florian Weimer (#4)
Re: possible design bug with PQescapeString()

* Tatsuo Ishii:

Users can input value for "var" from a web form. The attacker inputs
following string:

(0x95+0x27);DELETE FROM members;--

where 0x95+0x27 is actually a SJIS mutibyte KANJI. Programmer applies
PQescapeString() to it and gets:

0x95+0x27+0x27;DELETE FROM members;--

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126. Unfortunately, there is nothing we can do about
this in the current function because tha twould need four times the
lenggth of the input string (plus one). Drat.

Please don't do that. That would break all applications those use
the mutibyte encodings including UTF-8.

(I don't think you should have to consider the encoding in the client;
strange things may happen if there is an interpretation conflict
between the client and the backend.)

No. For the sake PQmblen() is provided. What I (and I guess Tom too)
am thinking is like this:

attacker's input:

(0x95+0x27);DELETE FROM members;--

new-PQescapeString() treats this:

0x95+0x27;DELETE FROM members;--

because the encoding is SJIS. And the result SQL will be:

SELECT * FROM members WHERE member_name = '0x95+0x27;DELETE FROM members;--';

The attacker loses.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#6Florian Weimer
fw@deneb.enyo.de
In reply to: Tatsuo Ishii (#5)
Re: possible design bug with PQescapeString()

* Tatsuo Ishii:

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126. Unfortunately, there is nothing we can do about
this in the current function because tha twould need four times the
lenggth of the input string (plus one). Drat.

Please don't do that. That would break all applications those use
the mutibyte encodings including UTF-8.

Why? Doesn't the server perform unquoting *before* multi-byte
processing? -- Ah, it doesn't. Perhaps this is the part which should
be fixed?

(I don't think you should have to consider the encoding in the client;
strange things may happen if there is an interpretation conflict
between the client and the backend.)

No. For the sake PQmblen() is provided. What I (and I guess Tom too)
am thinking is like this:

attacker's input:

(0x95+0x27);DELETE FROM members;--

new-PQescapeString() treats this:

0x95+0x27;DELETE FROM members;--

But this still needs knowledge of SJIS at the client side (and both
client and backend must have the same notion of SJIS).

#7Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Florian Weimer (#6)
Re: possible design bug with PQescapeString()

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126. Unfortunately, there is nothing we can do about
this in the current function because tha twould need four times the
lenggth of the input string (plus one). Drat.

Please don't do that. That would break all applications those use
the mutibyte encodings including UTF-8.

Why? Doesn't the server perform unquoting *before* multi-byte
processing? -- Ah, it doesn't. Perhaps this is the part which should
be fixed?

No no. Probably you misunderstand why we need quoting. If special
characters such as "'" or "\" appears, it should be quoted. But you
should not if it's a part of multibyte characters.

(I don't think you should have to consider the encoding in the client;
strange things may happen if there is an interpretation conflict
between the client and the backend.)

No. For the sake PQmblen() is provided. What I (and I guess Tom too)
am thinking is like this:

attacker's input:

(0x95+0x27);DELETE FROM members;--

new-PQescapeString() treats this:

0x95+0x27;DELETE FROM members;--

But this still needs knowledge of SJIS at the client side (and both
client and backend must have the same notion of SJIS).

No problem. We have the client encoding in PGConn. That's why Tom suggests
PQescapeString() should have the PGCConn argument.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Florian Weimer (#4)
Re: possible design bug with PQescapeString()

Florian Weimer <fw@deneb.enyo.de> writes:

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126.

No, that doesn't work, because the de-escaping on the backend side
happens *after* conversion to the backend encoding. If you insert escapes
into the middle of multibyte characters then you break the conversion.

Tatsuo's description of the problem is accurate (though I'm not sure
I agree with his solution ;-))

regards, tom lane

#9Martijn van Oosterhout
kleptog@svana.org
In reply to: Tom Lane (#8)
Re: possible design bug with PQescapeString()

On Sun, Feb 19, 2006 at 12:13:48PM -0500, Tom Lane wrote:

Florian Weimer <fw@deneb.enyo.de> writes:

Uh-oh, this is my fault. PQescapeString should escape all characters
greater than 126.

No, that doesn't work, because the de-escaping on the backend side
happens *after* conversion to the backend encoding. If you insert escapes
into the middle of multibyte characters then you break the conversion.

Well, most encodings provide an easy way to determine leader and
follower characters. The PQmblen() and related functions can help here.
Something like:

if( PQmblen(enc,ptr) > 1 )
copy bytes
else if( SQL_STR_DOUBLE( *ptr ) )
etc...

Assuming there are no multibyte string terminators... And assuming you
actually know what encoding the server expects.

However, the real solution seems to me to be to use something like
PQexecParams and ship the arguments outside the query string, thus
avoiding the issue entirely.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

Show quoted text

Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
tool for doing 5% of the work and then sitting around waiting for someone
else to do the other 95% so you can sue them.

#10Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Tatsuo Ishii (#3)
Re: possible design bug with PQescapeString()

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I suggest that PQescapeString() should have a parameter to specify the
encoding of "to".

You mean the encoding of "from", no?

Oops, "from", yes.

But actually I'd argue that
letting the client programmer supply the encoding is still a pretty
dangerous practice. Your example demonstrates that if the encoding
PQescapeString is told is different from the encoding the backend parser
thinks is in use, problems result. Perhaps we should pass the PGconn
to new-PQescapeString and let it dig the client encoding out of that.

Sound good to pass PGconn to new-PQescapeString. Here is the proposed
calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char *to, const char *from, size_t length)

If this is ok, I will implement for 8.2.

In further investigation, Akio Ishida found this kind of attack is
possible even with EUC_JP/UTF-8. So it seems the problem is not only
with SJIS, BIG5 and GBK. I think we need to add PQescapeStringWithConn
or whatever as soon as possible.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Show quoted text

You could still get burnt if the client encoding changes between the
invocation of new-PQescapeString and the sending of the constructed
command, but that's a fairly unlikely case.

The bottom line to this though is that these encodings are just plain
dangerous. I'm more than half tempted to suggest that the only secure
answer is to drop support for these encodings. Consider for example
an application that isn't using PQescapeString but has its own
double-backslashes-and-quotes logic embedded. You can break it if you
can manage to get the backend to think that the client encoding is SJIS
or similar. That's a hazard we're basically not ever going to be able
to prevent.

Dropping support for SJIS and so on will not be practical at all since
these encodings has been widely used and I don't see these encodings
are deprecated in the near future. I think dropping the support will
simply prevent people from using PostgreSQL. Especially in Windows
world, these encodings are pretty common.

I know that these encodings are broken in their design and actually I
hate them:-) But this is real world and we have to live with them...
--
Tatsuo Ishii
SRA OSS, Inc. Japan

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

#11Andrew - Supernews
andrew+nonews@supernews.com
In reply to: Tatsuo Ishii (#1)
Re: possible design bug with PQescapeString()

On 2006-02-20, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

In further investigation, Akio Ishida found this kind of attack is
possible even with EUC_JP/UTF-8.

How?

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

#12Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Andrew - Supernews (#11)
Re: possible design bug with PQescapeString()

On 2006-02-20, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

In further investigation, Akio Ishida found this kind of attack is
possible even with EUC_JP/UTF-8.

How?

The details have been sent to cores.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#13Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Tatsuo Ishii (#10)
1 attachment(s)
Re: possible design bug with PQescapeString()

But actually I'd argue that
letting the client programmer supply the encoding is still a pretty
dangerous practice. Your example demonstrates that if the encoding
PQescapeString is told is different from the encoding the backend parser
thinks is in use, problems result. Perhaps we should pass the PGconn
to new-PQescapeString and let it dig the client encoding out of that.

Sound good to pass PGconn to new-PQescapeString. Here is the proposed
calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char *to, const char *from, size_t length)

If this is ok, I will implement for 8.2.

Here is the promised patches for 8.2. If there's no objection, I will
commit tomorrow.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Attachments:

libpq.patchtext/plain; charset=us-asciiDownload
cvs diff: Diffing src/interfaces/libpq
Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.179
diff -c -r1.179 fe-exec.c
*** src/interfaces/libpq/fe-exec.c	25 Jan 2006 20:44:32 -0000	1.179
--- src/interfaces/libpq/fe-exec.c	26 Feb 2006 14:16:30 -0000
***************
*** 2373,2378 ****
--- 2373,2435 ----
  }
  
  /*
+  * Escaping arbitrary strings to get valid SQL literal strings.
+  * mostly same as PQescapeString() except that this function is
+  * multibyte aware. The encoding info is retrieved from conn. So you
+  * should set proper client encoding before using this.
+  *
+  * Replaces "\\" with "\\\\" and "'" with "''".
+  *
+  * length is the length of the source string.  (Note: if a terminating NUL
+  * is encountered sooner, PQescapeString stops short of "length"; the behavior
+  * is thus rather like strncpy.)
+  *
+  * For safety the buffer at "to" must be at least 2*length + 1 bytes long.
+  * A terminating NUL character is added to the output string, whether the
+  * input is NUL-terminated or not.
+  *
+  * Returns the actual length of the output (not counting the terminating NUL).
+  */
+ size_t
+ PQescapeStringWithConn(PGconn *conn, char *to, const char *from, size_t length)
+ {
+ 	const char *source = from;
+ 	char	   *target = to;
+ 	size_t		remaining = length;
+ 	int len;
+ 
+ 	if (!conn)
+ 	{
+ 		*target = '\0';
+ 		return 0;
+ 	}
+ 
+ 	while (remaining > 0 && *source != '\0')
+ 	{
+ 		if (SQL_STR_DOUBLE(*source))
+ 		{
+ 			*target++ = *source;
+ 			*target++ = *source++;
+ 		}
+ 		else
+ 		{
+ 			len = PQmblen(source, conn->client_encoding);
+ 			while (*source != '\0' && remaining > 0 && len > 0)
+ 			{
+ 				*target++ = *source++;
+ 				remaining--;
+ 				len--;
+ 			}
+ 		}
+ 	}
+ 
+ 	/* Write the terminating NUL character. */
+ 	*target = '\0';
+ 
+ 	return target - to;
+ }
+ 
+ /*
   *		PQescapeBytea	- converts from binary string to the
   *		minimal encoding necessary to include the string in an SQL
   *		INSERT statement with a bytea type column as the target.
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.124
diff -c -r1.124 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h	26 Dec 2005 14:58:06 -0000	1.124
--- src/interfaces/libpq/libpq-fe.h	26 Feb 2006 14:16:30 -0000
***************
*** 436,441 ****
--- 436,442 ----
  
  /* Quoting strings before inclusion in queries. */
  extern size_t PQescapeString(char *to, const char *from, size_t length);
+ extern size_t PQescapeStringWithConn(PGconn *conn, char *to, const char *from, size_t length);
  extern unsigned char *PQescapeBytea(const unsigned char *bintext, size_t binlen,
  			  size_t *bytealen);
  extern unsigned char *PQunescapeBytea(const unsigned char *strtext,
cvs diff: Diffing src/interfaces/libpq/po
cvs diff: Diffing doc/src/sgml
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.201
diff -c -r1.201 libpq.sgml
*** doc/src/sgml/libpq.sgml	26 Dec 2005 14:58:04 -0000	1.201
--- doc/src/sgml/libpq.sgml	26 Feb 2006 14:16:34 -0000
***************
*** 2253,2258 ****
--- 2253,2323 ----
  </para>
  </sect2>
  
+ <sect2 id="libpq-exec-escape-string-with-conn">
+   <title>Escaping Strings for Inclusion in SQL Commands</title>
+ 
+    <indexterm zone="libpq-exec-escape-string-with-conn"><primary>PQescapeStringWithConn</></>
+    <indexterm zone="libpq-exec-escape-string-with-conn"><primary>escaping strings</></>
+ 
+ <para>
+ <function>PQescapeStringWithConn</function> escapes a string for use within an SQL
+ command.  This is useful when inserting data values as literal constants
+ in SQL commands.  Certain characters (such as quotes and backslashes) must
+ be escaped to prevent them from being interpreted specially by the SQL parser.
+ <function>PQescapeStringWithConn</> performs this operation.
+ </para>
+ 
+ <tip>
+ <para>
+ It is especially important to do proper escaping when handling strings that
+ were received from an untrustworthy source.  Otherwise there is a security
+ risk: you are vulnerable to <quote>SQL injection</> attacks wherein unwanted
+ SQL commands are fed to your database.
+ </para>
+ </tip>
+ 
+ <para>
+ Note that it is not necessary nor correct to do escaping when a data
+ value is passed as a separate parameter in <function>PQexecParams</> or
+ its sibling routines.
+ 
+ <synopsis>
+ size_t PQescapeStringWithConn (PGconn *conn, char *to, const char *from, size_t length);
+ </synopsis>
+ </para>
+ 
+ <para>
+ The parameter <parameter>conn</> is the existing DB connection handle.
+ Before using this function it is important to set the client encoding
+ by using <literal>SET client_encoding TO ...</literal> if neccessary.
+ The parameter <parameter>from</> points to the first character of the string
+ that is to be escaped, and the <parameter>length</> parameter gives the
+ number of characters in this string.  A terminating zero byte is not
+ required, and should not be counted in <parameter>length</>.  (If
+ a terminating zero byte is found before <parameter>length</> bytes are
+ processed, <function>PQescapeStringWithConn</> stops at the zero; the behavior
+ is thus rather like <function>strncpy</>.)
+ <parameter>to</> shall point to a
+ buffer that 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>PQescapeStringWithConn</> 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 zero byte.  The single quotes that
+ must surround <productname>PostgreSQL</> string literals are not
+ included in the result string; they should be provided in the SQL
+ command that the result is inserted into.
+ </para>
+ <para>
+ <function>PQescapeStringWithConn</> returns the number of characters written
+ to <parameter>to</>, not including the terminating zero byte.
+ </para>
+ <para>
+ Behavior is undefined if the <parameter>to</> and <parameter>from</>
+ strings overlap.
+ </para>
+ </sect2>
+ 
  
   <sect2 id="libpq-exec-escape-bytea">
    <title>Escaping Binary Strings for Inclusion in SQL Commands</title>
#14Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Tatsuo Ishii (#13)
Re: possible design bug with PQescapeString()

But actually I'd argue that
letting the client programmer supply the encoding is still a
pretty dangerous practice. Your example demonstrates

that if the

encoding PQescapeString is told is different from the

encoding the

backend parser thinks is in use, problems result. Perhaps we
should pass the PGconn to new-PQescapeString and let it

dig the client encoding out of that.

Sound good to pass PGconn to new-PQescapeString. Here is the
proposed calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char

*to, const

char *from, size_t length)

If this is ok, I will implement for 8.2.

Here is the promised patches for 8.2. If there's no
objection, I will commit tomorrow.

Just a reminder - to work on win32, the new function needs an entry in
exports.txt.

Thanks. Could you tell me what the number column in the file is? Can I
assign arbitary number for the function? (maybe next to the last
number?) I'm not familiar with win32 build.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#15Magnus Hagander
mha@sollentuna.net
In reply to: Tatsuo Ishii (#14)
Re: possible design bug with PQescapeString()

But actually I'd argue that
letting the client programmer supply the encoding is still a
pretty dangerous practice. Your example demonstrates

that if the

encoding PQescapeString is told is different from the

encoding the

backend parser thinks is in use, problems result. Perhaps we
should pass the PGconn to new-PQescapeString and let it

dig the client encoding out of that.

Sound good to pass PGconn to new-PQescapeString. Here is the
proposed calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char

*to, const

char *from, size_t length)

If this is ok, I will implement for 8.2.

Here is the promised patches for 8.2. If there's no
objection, I will commit tomorrow.

Just a reminder - to work on win32, the new function needs an entry in
exports.txt.

//Magnus

#16Magnus Hagander
mha@sollentuna.net
In reply to: Magnus Hagander (#15)
Re: possible design bug with PQescapeString()

Sound good to pass PGconn to new-PQescapeString. Here is the
proposed calling sequence for the new function:

size_t PQescapeStringWithConn (const PGconn *conn, char

*to, const

char *from, size_t length)

If this is ok, I will implement for 8.2.

Here is the promised patches for 8.2. If there's no objection, I
will commit tomorrow.

Just a reminder - to work on win32, the new function needs

an entry in

exports.txt.

Thanks. Could you tell me what the number column in the file
is? Can I assign arbitary number for the function? (maybe
next to the last
number?) I'm not familiar with win32 build.

It just has to be unique, and never change for each function. So just
assign the next one in sequence.

//Magnus

#17Andrew - Supernews
andrew+nonews@supernews.com
In reply to: Tatsuo Ishii (#3)
Re: possible design bug with PQescapeString()

On 2006-02-26, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

On 2006-02-20, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

In further investigation, Akio Ishida found this kind of attack is
possible even with EUC_JP/UTF-8.

How?

The details have been sent to cores.

I wasn't asking out of idle curiosity. Some preliminary investigation
that I have done suggests that when using UTF-8, the proposed changes
do not fix the problem (and may make matters worse). So I want to know
whether the problem that I'm looking at is the same thing as the one
you're looking at.

UTF-8 has the property that neither ' nor \ can appear as part of a
valid multibyte sequence. But many places in postgres are extremely
sloppy about handling _invalid_ utf-8, and unless you're prepared to
make the escape routine fail outright in such cases (which I would
strongly favour), it is likely that there will always be ways to get
malformed sequences into the backend (which itself is far too lax
about parsing them).

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

#18Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Andrew - Supernews (#17)
Re: possible design bug with PQescapeString()

On 2006-02-26, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

On 2006-02-20, Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

In further investigation, Akio Ishida found this kind of attack is
possible even with EUC_JP/UTF-8.

How?

The details have been sent to cores.

I wasn't asking out of idle curiosity. Some preliminary investigation
that I have done suggests that when using UTF-8, the proposed changes
do not fix the problem (and may make matters worse). So I want to know
whether the problem that I'm looking at is the same thing as the one
you're looking at.

UTF-8 has the property that neither ' nor \ can appear as part of a
valid multibyte sequence. But many places in postgres are extremely
sloppy about handling _invalid_ utf-8, and unless you're prepared to
make the escape routine fail outright in such cases (which I would
strongly favour), it is likely that there will always be ways to get
malformed sequences into the backend (which itself is far too lax
about parsing them).

I guess I understand whay you are saying. However, I am not allowed to
talk to you about it unless cores allow me. Probably we need some
closed forum to discuss this kind of security issues. The forum should
be consisted with cores and people who are admitted by cores.

Cores, what do you think?
--
Tatsuo Ishii
SRA OSS, Inc. Japan

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tatsuo Ishii (#18)
Re: possible design bug with PQescapeString()

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I guess I understand whay you are saying. However, I am not allowed to
talk to you about it unless cores allow me. Probably we need some
closed forum to discuss this kind of security issues.

Considering that you've already described the problem on pgsql-hackers,
I hardly see how further discussion is going to create a bigger security
breach than already exists.

(I'm of the opinion that the problem is mostly a client problem anyway;
AFAICS the issue only comes up if client software fails to consider
encoding issues while doing escaping. There is certainly no way that
we can magically solve the problem in a new PG release, and so trying
to keep it quiet until we can work out a solution seems pointless.)

regards, tom lane

#20Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Tom Lane (#19)
Re: possible design bug with PQescapeString()

FYI

I have sent an email to cores to ask if I am OK to bring another but
closely related to this issue to open discussions, whose details have
already been sent to them. The reason why I'm asking is, if this issue
could be open, then the issue might be open too and that makes
discussions easier.

At this point, I get no response from them so far.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Show quoted text

Tatsuo Ishii <ishii@sraoss.co.jp> writes:

I guess I understand whay you are saying. However, I am not allowed to
talk to you about it unless cores allow me. Probably we need some
closed forum to discuss this kind of security issues.

Considering that you've already described the problem on pgsql-hackers,
I hardly see how further discussion is going to create a bigger security
breach than already exists.

(I'm of the opinion that the problem is mostly a client problem anyway;
AFAICS the issue only comes up if client software fails to consider
encoding issues while doing escaping. There is certainly no way that
we can magically solve the problem in a new PG release, and so trying
to keep it quiet until we can work out a solution seems pointless.)

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend