[libpostgresql-jdbc-java] 01/01: Backport to 7.3. 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.

[Emmanuel Bourg]

This is an automated email from the git hooks/post-receive script.

ebourg-guest pushed a commit to tag REL7_3_5
in repository libpostgresql-jdbc-java.

commit b7556b365129f5076b40e8721a0b0318d4fb5ac3
Author: Barry Lind <barry at xythos.com>
Date:   Thu Aug 7 17:56:27 2003 +0000

    Backport to 7.3.  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:
      Tag: REL7_3_STABLE
    	jdbc/org/postgresql/Driver.java.in
     	jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java
---
 org/postgresql/Driver.java.in                    |  2 +-
 org/postgresql/jdbc1/AbstractJdbc1Statement.java | 36 +++++++++++-------------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/org/postgresql/Driver.java.in b/org/postgresql/Driver.java.in
index 241c588..c53f2fd 100644
--- a/org/postgresql/Driver.java.in
+++ b/org/postgresql/Driver.java.in
@@ -446,6 +446,6 @@ public class Driver implements java.sql.Driver
 	}
 
 	//The build number should be incremented for every new build
-	private static int m_buildNumber = 112;
+	private static int m_buildNumber = 113;
 
 }
diff --git a/org/postgresql/jdbc1/AbstractJdbc1Statement.java b/org/postgresql/jdbc1/AbstractJdbc1Statement.java
index 925fc06..337ecef 100644
--- a/org/postgresql/jdbc1/AbstractJdbc1Statement.java
+++ b/org/postgresql/jdbc1/AbstractJdbc1Statement.java
@@ -914,25 +914,13 @@ public abstract class AbstractJdbc1Statement implements org.postgresql.PGStateme
 				sbuf.setLength(0);
 				sbuf.ensureCapacity(x.length());
 				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();
-		}
-	}
-
 	/*
 	 * p_allowStatementTerminator determines if a semi-colon is allowed in the
 	 * returned value.  A semi-colon should only be allowed if the resulting 
@@ -940,7 +928,7 @@ public abstract class AbstractJdbc1Statement implements org.postgresql.PGStateme
 	 * passed by value to the server via a bind thus bypassing the sql parser
 	 * on the server.
 	 */
-	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);
@@ -953,9 +941,6 @@ public abstract class AbstractJdbc1Statement implements org.postgresql.PGStateme
 					break;
 			    case '\0':
 					throw new IllegalArgumentException("\\0 not allowed");
-			    case ';':
-					if (!p_allowStatementTerminator)
-						throw new IllegalArgumentException("semicolon not allowed");
 				default:
 					p_output.append(c);
 			}
@@ -1375,7 +1360,14 @@ public abstract class AbstractJdbc1Statement implements org.postgresql.PGStateme
 		switch (targetSqlType)
 		{
 			case Types.INTEGER:
-				bind(parameterIndex, escapeString(x.toString()), PG_INTEGER);
+				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
+					//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:
@@ -1387,8 +1379,14 @@ public abstract class AbstractJdbc1Statement implements org.postgresql.PGStateme
 			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:

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-java/libpostgresql-jdbc-java.git