From b9b3777671c8a5cc580e1985f61337d39d47c730 Mon Sep 17 00:00:00 2001
From: Dave Cramer <davecramer@gmail.com>
Date: Mon, 19 Feb 2024 08:20:59 -0500
Subject: [PATCH] Merge pull request from GHSA-24rp-q3w6-vc56

* SQL Injection via line comment generation for 42_2_x

* fix: Add parentheses around NULL parameter values in simple query mode

* simplify code, handle binary and add tests

* remove extra spaces

---------

Co-authored-by: Sehrope Sarkuni <sehrope@jackdb.com>
---
 .../core/v3/SimpleParameterList.java          | 118 ++++++++++++------
 .../core/v3/V3ParameterListTests.java         |   6 +-
 .../jdbc/ParameterInjectionTest.java          |  67 ++++++++++
 3 files changed, 149 insertions(+), 42 deletions(-)
 create mode 100644 pgjdbc/src/test/java/org/postgresql/jdbc/ParameterInjectionTest.java

Index: postgresql-jdbc-9.4-1201.src/org/postgresql/core/v3/SimpleParameterList.java
===================================================================
--- postgresql-jdbc-9.4-1201.src.orig/org/postgresql/core/v3/SimpleParameterList.java
+++ postgresql-jdbc-9.4-1201.src/org/postgresql/core/v3/SimpleParameterList.java
@@ -14,6 +14,8 @@ import java.sql.SQLException;
 import java.util.Arrays;
 
 import org.postgresql.core.*;
+import org.postgresql.geometric.PGbox;
+import org.postgresql.geometric.PGpoint;
 import org.postgresql.util.PSQLException;
 import org.postgresql.util.PSQLState;
 import org.postgresql.util.StreamWrapper;
@@ -146,79 +148,146 @@ class SimpleParameterList implements V3P
         bind(index, NULL_OBJECT, oid, binaryTransfer);
     }
 
+    /**
+     * <p>Escapes a given text value as a literal, wraps it in single quotes, casts it to the
+     * to the given data type, and finally wraps the whole thing in parentheses.</p>
+     *
+     * <p>For example, "123" and "int4" becomes "('123'::int)"</p>
+     *
+     * <p>The additional parentheses is added to ensure that the surrounding text of where the
+     * parameter value is entered does modify the interpretation of the value.</p>
+     *
+     * <p>For example if our input SQL is: <code>SELECT ?b</code></p>
+     *
+     * <p>Using a parameter value of '{}' and type of json we'd get:</p>
+     *
+     * <pre>
+     * test=# SELECT ('{}'::json)b;
+     *  b
+     * ----
+     *  {}
+     * </pre>
+     *
+     * <p>But without the parentheses the result changes:</p>
+     *
+     * <pre>
+     * test=# SELECT '{}'::jsonb;
+     * jsonb
+     * -------
+     * {}
+     * </pre>
+     **/
+    private static String quoteAndCast(String text, /* @Nullable */ String type, boolean standardConformingStrings) {
+      StringBuilder sb = new StringBuilder((text.length() + 10) / 10 * 11); // Add 10% for escaping.
+      sb.append("('");
+      try {
+        Utils.escapeLiteral(sb, text, standardConformingStrings);
+      } catch (SQLException e) {
+        // This should only happen if we have an embedded null
+        // and there's not much we can do if we do hit one.
+        //
+        // To force a server side failure, we deliberately include
+        // a zero byte character in the literal to force the server
+        // to reject the command.
+        sb.append('\u0000');
+      }
+      sb.append("'");
+      if (type != null) {
+        sb.append("::");
+        sb.append(type);
+      }
+      sb.append(")");
+      return sb.toString();
+    }
+
     public String toString(int index) {
         --index;
         if (paramValues[index] == null)
             return "?";
         else if (paramValues[index] == NULL_OBJECT)
-            return "NULL";
+            return "(NULL)";
 
-        else if ( (flags[index]& BINARY) == BINARY )
-        {
+        String textValue;
+        String type;
+        if ((flags[index] & BINARY) == BINARY) {
             // handle some of the numeric types
-
             switch (paramTypes[index])
             {
                 case Oid.INT2:
                     short s = ByteConverter.int2((byte[])paramValues[index],0);
-                    return Short.toString(s);
+                    textValue = Short.toString(s);
+                    type = "int2";
+                    break;
 
                 case Oid.INT4:
                     int i = ByteConverter.int4((byte[])paramValues[index],0);
-                    return Integer.toString(i);
+                    textValue = Integer.toString(i);
+                    type = "int4";
+                    break;
 
                 case Oid.INT8:
                     long l = ByteConverter.int8((byte[])paramValues[index],0);
-                    return Long.toString(l);
+                    textValue = Long.toString(l);
+                    type = "int8";
+                    break;
 
                 case Oid.FLOAT4:
                     float f = ByteConverter.float4((byte[])paramValues[index],0);
-                    return Float.toString(f);
+                    textValue = Float.toString(f);
+                    type = "real";
+                    break;
 
                 case Oid.FLOAT8:
                     double d = ByteConverter.float8((byte[])paramValues[index],0);
-                    return Double.toString(d);
+                    textValue = Double.toString(d);
+                    type = "double precision";
+                    break;
+
+		case Oid.POINT:
+                    PGpoint pgPoint = new PGpoint();
+                    pgPoint.setByteValue((byte[]) paramValues[index], 0);
+                    textValue = pgPoint.toString();
+                    type = "point";
+		    break;
+
+		case Oid.BOX:
+		    PGbox pgBox = new PGbox();
+		    pgBox.setByteValue((byte[]) paramValues[index], 0);
+                    textValue = pgBox.toString();
+                    type = "box";
+                    break;
+
+                default:
+                    return "?";
             }
-            return "?";
         }
         else
         {
-            String param = paramValues[index].toString();
-            boolean hasBackslash = param.indexOf('\\') != -1;
-
-            // add room for quotes + potential escaping.
-            StringBuilder p = new StringBuilder(3 + param.length() * 11 / 10);
-
-            boolean standardConformingStrings = false;
-            boolean supportsEStringSyntax = false;
-            if (protoConnection != null)
-            {
-                standardConformingStrings = protoConnection.getStandardConformingStrings();
-                supportsEStringSyntax = protoConnection.getServerVersionNum() >= 80100;
+            textValue = paramValues[index].toString();
+            int paramType = paramTypes[index];
+            if (paramType == Oid.TIMESTAMP) {
+              type = "timestamp";
+            } else if (paramType == Oid.TIMESTAMPTZ) {
+              type = "timestamp with time zone";
+            } else if (paramType == Oid.TIME) {
+              type = "time";
+            } else if (paramType == Oid.TIMETZ) {
+              type = "time with time zone";
+            } else if (paramType == Oid.DATE) {
+              type = "date";
+            } else if (paramType == Oid.INTERVAL) {
+              type = "interval";
+            } else if (paramType == Oid.NUMERIC) {
+              type = "numeric";
+            } else {
+              type = null;
             }
-
-            if (hasBackslash && !standardConformingStrings && supportsEStringSyntax)
-                p.append('E');
-
-            p.append('\'');
-            try
-            {
-                p = Utils.escapeLiteral(p, param, standardConformingStrings);
-            }
-            catch (SQLException sqle)
-            {
-                // This should only happen if we have an embedded null
-                // and there's not much we can do if we do hit one.
-                //
-                // The goal of toString isn't to be sent to the server,
-                // so we aren't 100% accurate (see StreamWrapper), put
-                // the unescaped version of the data.
-                //
-                p.append(param);
-            }
-            p.append('\'');
-            return p.toString();
         }
+        boolean standardConformingStrings = false;
+        if (protoConnection != null) {
+            standardConformingStrings = protoConnection.getStandardConformingStrings();
+        }
+        return quoteAndCast(textValue, type, standardConformingStrings);
     }
 
     public void checkAllParametersSet() throws SQLException {
@@ -226,8 +295,8 @@ class SimpleParameterList implements V3P
         {
             if (direction(i) != OUT && paramValues[i] == null)
                 throw new PSQLException(GT.tr("No value specified for parameter {0}.", new Integer(i + 1)), PSQLState.INVALID_PARAMETER_VALUE);
-            }
         }
+    }
 
     public void convertFunctionOutParameters()
     {
