Escaping strings for inclusion into SQL queries

Escaping strings for inclusion into SQL queries

Post by Bruce Momji » Sun, 09 Sep 2001 05:27:54



Has this been resolved?


> > 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

  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

 
 
 

Escaping strings for inclusion into SQL queries

Post by Tom La » Sun, 09 Sep 2001 05:50:07



> 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

---------------------------(end of broadcast)---------------------------


 
 
 

Escaping strings for inclusion into SQL queries

Post by Bruce Momji » Sun, 09 Sep 2001 06:36:41


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 <pg...@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.999562...@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.Wei...@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 majord...@postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pg...@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

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

 
 
 

Escaping strings for inclusion into SQL queries

Post by Bruce Momji » Sun, 09 Sep 2001 07:30:00


Patch applied.  Thanks.


> > 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

> use identifiers from an untrusted source at all.  Therefore, all
> claims of the suitability of PQescapeString() for identifiers have
> been removed.

> --

> 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

>  </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

>  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

>  /* === 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)---------------------------


--
  Bruce Momjian                        |  http://candle.pha.pa.us

  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

 
 
 

Escaping strings for inclusion into SQL queries

Post by Joe Conwa » Wed, 12 Sep 2001 15:40:37


> Patch applied.  Thanks.


> > > 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

> > 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

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate

message can get through to the mailing list cleanly

 
 
 

Escaping strings for inclusion into SQL queries

Post by Florian Weim » Wed, 12 Sep 2001 23:56:26


--=-=-=


> 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.

Quote:> 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.

--

University of Stuttgart           http://cert.uni-stuttgart.de/
RUS-CERT                          +49-711-685-5973/fax +49-711-685-5898

--=-=-=
Content-Disposition: attachment

Index: fe-exec.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.110
diff -u -r1.110 fe-exec.c
--- fe-exec.c   2001/09/07 22:02:32     1.110

 /* ---------------
  * Escaping arbitrary strings to get valid SQL strings/identifiers.
  *
- * Replaces "\\" with "\\\\", "\0" with "\\0", and "'" with "''".
+ * Replaces "\\" with "\\\\" 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

        while (remaining > 0) {
                switch (*source) {
-               case '\0':
-                       *target = '\\';
-                       target++;
-                       *target = '0';
-                       /* target and remaining are updated below. */
-                       break;
-                      
                case '\\':
                        *target = '\\';
                        target++;

--=-=-=
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: binary
MIME-Version: 1.0

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate

message can get through to the mailing list cleanly

--=-=-=--