Patch applied for SQL Injection vulnerability for setObject(int,Object,int) - Mailing list pgsql-jdbc

I have applied a fix for the reported SQL injection vulnerability in
setObject for both 7.3 and 7.4.  I have also places new builds with this
bugfix on the jdbc.postgresql.org website.

For the record, the vulnerability was limited to the
setObject(int,Object,int) method where Object is a String and the type
is a numeric type (i.e. SqlTypes.INTEGER, SqlTypes.LONG, etc)

Given the ongoing discussion that this SQL injection vulnerability has
caused, I decided not to apply the below patch from Kim and instead
fixed the problem in a different way.  The fix essentially applies the
regular escaping done for setString to appropriate values passed to
setObject.  It does not however add quotes to the value.  Thus existing
uses of setObject for in clause and array type values will still
continue to work.

I didn't want to break backward compatibility in a patch to 7.3 and
didn't want to change the functionality in 7.4 until the current
discussions come to a conclusion.

thanks,
--Barry

PS.  I have not yet uploaded new jdbc1 builds to the website.  As I have
recently upgraded to RH9, my jdk1.1 environment no longer works and I
will need to do this build on a different machine tomorrow.


Kim Ho wrote:
> To speed things up a bit, since the regoutParam patch is not likely to
> be approved anytime soon.
>
> This patch
> - adds single quotes for numbers in setObject and also setInt/Byte/etc.
> - Improves getInt/Long when you may have parser errors if you're too
> close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
> - Improves radix point handling when using setObject to an integer
> parameter while passing in a float. This is especially important in
> callable statements.
>
> Cheers,
>
> Kim
>
> On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>>Barry Lind wrote:
>>
>>>Dmitry,
>>>
>>>That is a bug.  Thanks for pointing it out.  Anyone care to submit a patch?
>>>
>>
>>Kim's patch fixes this.  It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario   M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
>
>
> ------------------------------------------------------------------------
>
> ? temp.diff
> Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> retrieving revision 1.13
> diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    30 Jun 2003 21:10:55 -0000    1.13
> --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java    18 Jul 2003 17:02:20 -0000
> *************** public abstract class AbstractJdbc1Resul
> *** 805,811 ****
>               try
>               {
>                   s = s.trim();
> !                 return Integer.parseInt(s);
>               }
>               catch (NumberFormatException e)
>               {
> --- 805,811 ----
>               try
>               {
>                   s = s.trim();
> !                 return Float.valueOf(s).intValue();
>               }
>               catch (NumberFormatException e)
>               {
> *************** public abstract class AbstractJdbc1Resul
> *** 822,828 ****
>               try
>               {
>                   s = s.trim();
> !                 return Long.parseLong(s);
>               }
>               catch (NumberFormatException e)
>               {
> --- 822,828 ----
>               try
>               {
>                   s = s.trim();
> !                 return Double.valueOf(s).longValue();
>               }
>               catch (NumberFormatException e)
>               {
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.27
> diff -c -p -r1.27 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java    9 Jul 2003 05:12:04 -0000    1.27
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java    18 Jul 2003 17:02:22 -0000
> *************** public abstract class AbstractJdbc1State
> *** 920,926 ****
>        */
>       public void setByte(int parameterIndex, byte x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_TEXT);
>       }
>
>       /*
> --- 920,926 ----
>        */
>       public void setByte(int parameterIndex, byte x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 933,939 ****
>        */
>       public void setShort(int parameterIndex, short x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_INT2);
>       }
>
>       /*
> --- 933,939 ----
>        */
>       public void setShort(int parameterIndex, short x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 946,952 ****
>        */
>       public void setInt(int parameterIndex, int x) throws SQLException
>       {
> !         bind(parameterIndex, Integer.toString(x), PG_INTEGER);
>       }
>
>       /*
> --- 946,952 ----
>        */
>       public void setInt(int parameterIndex, int x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 959,965 ****
>        */
>       public void setLong(int parameterIndex, long x) throws SQLException
>       {
> !         bind(parameterIndex, Long.toString(x), PG_INT8);
>       }
>
>       /*
> --- 959,965 ----
>        */
>       public void setLong(int parameterIndex, long x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 972,978 ****
>        */
>       public void setFloat(int parameterIndex, float x) throws SQLException
>       {
> !         bind(parameterIndex, Float.toString(x), PG_FLOAT);
>       }
>
>       /*
> --- 972,978 ----
>        */
>       public void setFloat(int parameterIndex, float x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 985,991 ****
>        */
>       public void setDouble(int parameterIndex, double x) throws SQLException
>       {
> !         bind(parameterIndex, Double.toString(x), PG_DOUBLE);
>       }
>
>       /*
> --- 985,991 ----
>        */
>       public void setDouble(int parameterIndex, double x) throws SQLException
>       {
> !         bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
>       }
>
>       /*
> *************** public abstract class AbstractJdbc1State
> *** 1003,1009 ****
>               setNull(parameterIndex, Types.DECIMAL);
>           else
>           {
> !             bind(parameterIndex, x.toString(), PG_NUMERIC);
>           }
>       }
>
> --- 1003,1009 ----
>               setNull(parameterIndex, Types.DECIMAL);
>           else
>           {
> !             bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
>           }
>       }
>
> *************** public abstract class AbstractJdbc1State
> *** 1464,1486 ****
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
> -                 if (x instanceof Boolean)
> -                     bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> -                 else
> -                     bind(parameterIndex, x.toString(), PG_INTEGER);
> -                 break;
>               case Types.TINYINT:
>               case Types.SMALLINT:
>               case Types.BIGINT:
>               case Types.REAL:
>               case Types.FLOAT:
>               case Types.DOUBLE:
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 if (x instanceof Boolean)
> !                     bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> !                 else
> !                     bind(parameterIndex, x.toString(), PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> --- 1464,1484 ----
>           switch (targetSqlType)
>           {
>               case Types.INTEGER:
>               case Types.TINYINT:
>               case Types.SMALLINT:
> +                 x = removeRadix(x,Types.INTEGER);
> +                 bindNumber(parameterIndex,x,PG_INTEGER);
> +                 break;
>               case Types.BIGINT:
> +                 x = removeRadix(x,Types.BIGINT);
> +                 bindNumber(parameterIndex,x,PG_INT8);
> +                 break;
>               case Types.REAL:
>               case Types.FLOAT:
>               case Types.DOUBLE:
>               case Types.DECIMAL:
>               case Types.NUMERIC:
> !                 bindNumber(parameterIndex,x,PG_NUMERIC);
>                   break;
>               case Types.CHAR:
>               case Types.VARCHAR:
> *************** public abstract class AbstractJdbc1State
> *** 2026,2031 ****
> --- 2024,2056 ----
>           if (parameterIndex != 1)
>               throw new PSQLException("postgresql.call.noinout");
>       }
> +
> +     private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> +     {
> +         if (x instanceof Boolean)
> +             bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> +         else
> +             bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> +     }
> +
> +
> +     private Object removeRadix(Object x, int sqlType)
> +     {
> +         if (x.toString().indexOf(".")>0)
> +         {
> +             switch (sqlType)
> +             {
> +                 case Types.BIGINT:
> +                     x = String.valueOf(Double.valueOf(x.toString()).longValue());
> +                     break;
> +                 default:
> +                     x = String.valueOf(Float.valueOf(x.toString()).intValue());
> +                     break;
> +             }
> +         }
> +         return x;
> +     }
> +
>
>
>
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(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