From acf09c64b0264715a4e29d3e04b5ca274350765a Mon Sep 17 00:00:00 2001 From: Barry Lind Date: Thu, 7 Aug 2003 04:03:13 +0000 Subject: [PATCH] Sometimes the third time is the charm. Third try to fix the sql injection vulnerability. This fix completely removes the ability (hack) of being able to bind a list of values in an in clause. It was demonstrated that by allowing that functionality you open up the possibility for certain types of sql injection attacks. The previous fix attempts all focused on preventing the insertion of additional sql statements (the semi-colon problem: xxx; any new sql statement here). But that still left the ability to change the where clause on the current statement or perform a subselect which can circumvent applicaiton security logic and/or allow you to call any stored function. Modified Files: jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java --- .../jdbc1/AbstractJdbc1Statement.java | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java b/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java index 9d4407399e..8b3c37d6ad 100644 --- a/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java +++ b/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java @@ -25,7 +25,7 @@ import java.sql.Timestamp; import java.sql.Types; import java.util.Vector; -/* $Header: /cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc1/Attic/AbstractJdbc1Statement.java,v 1.29 2003/07/24 00:30:39 barry Exp $ +/* $Header: /cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc1/Attic/AbstractJdbc1Statement.java,v 1.30 2003/08/07 04:03:13 barry Exp $ * This class defines methods of the jdbc1 specification. This class is * extended by org.postgresql.jdbc2.AbstractJdbc2Statement which adds the jdbc2 * methods. The real Statement class (for jdbc1) is org.postgresql.jdbc1.Jdbc1Statement @@ -1036,26 +1036,14 @@ public abstract class AbstractJdbc1Statement implements BaseStatement sbuf.setLength(0); sbuf.ensureCapacity(x.length() + (int)(x.length() / 10)); sbuf.append('\''); - escapeString(x, sbuf, true); + escapeString(x, sbuf); sbuf.append('\''); bind(parameterIndex, sbuf.toString(), type); } } } - private String escapeString(String p_input) { - // use the shared buffer object. Should never clash but this makes - // us thread safe! - synchronized (sbuf) - { - sbuf.setLength(0); - sbuf.ensureCapacity(p_input.length()); - escapeString(p_input, sbuf, false); - return sbuf.toString(); - } - } - - private void escapeString(String p_input, StringBuffer p_output, boolean p_allowStatementTerminator) { + private void escapeString(String p_input, StringBuffer p_output) { for (int i = 0 ; i < p_input.length() ; ++i) { char c = p_input.charAt(i); @@ -1068,9 +1056,6 @@ public abstract class AbstractJdbc1Statement implements BaseStatement break; case '\0': throw new IllegalArgumentException("\\0 not allowed"); - case ';': - if (!p_allowStatementTerminator) - throw new IllegalArgumentException("semicolon not allowed"); default: p_output.append(c); } @@ -1493,8 +1478,14 @@ public abstract class AbstractJdbc1Statement implements BaseStatement case Types.INTEGER: if (x instanceof Boolean) bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN); + else if (x instanceof Integer || x instanceof Long || + x instanceof Double || x instanceof Short || + x instanceof Number || x instanceof Float ) + bind(parameterIndex, x.toString(), PG_INTEGER); else - bind(parameterIndex, escapeString(x.toString()), PG_INTEGER); + //ensure the value is a valid numeric value to avoid + //sql injection attacks + bind(parameterIndex, new BigDecimal(x.toString()).toString(), PG_INTEGER); break; case Types.TINYINT: case Types.SMALLINT: @@ -1506,8 +1497,14 @@ public abstract class AbstractJdbc1Statement implements BaseStatement case Types.NUMERIC: if (x instanceof Boolean) bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN); + else if (x instanceof Integer || x instanceof Long || + x instanceof Double || x instanceof Short || + x instanceof Number || x instanceof Float ) + bind(parameterIndex, x.toString(), PG_NUMERIC); else - bind(parameterIndex, escapeString(x.toString()), PG_NUMERIC); + //ensure the value is a valid numeric value to avoid + //sql injection attacks + bind(parameterIndex, new BigDecimal(x.toString()).toString(), PG_NUMERIC); break; case Types.CHAR: case Types.VARCHAR: