Skip to content

Commit 990d63f

Browse files
davecramersehrope
davecramer
and
sehrope
authored
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>
1 parent b9b3777 commit 990d63f

File tree

2 files changed

+164
-64
lines changed

2 files changed

+164
-64
lines changed

‎pgjdbc/src/main/java/org/postgresql/core/v3/SimpleParameterList.java‎

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public void setNull(@Positive int index, int oid) throws SQLException {
201201
* {}
202202
* </pre>
203203
**/
204-
private static String quoteAndCast(String text, String type, boolean standardConformingStrings) {
204+
private static String quoteAndCast(String text, @Nullable String type, boolean standardConformingStrings) {
205205
StringBuilder sb = new StringBuilder((text.length() + 10) / 10 * 11); // Add 10% for escaping.
206206
sb.append("('");
207207
try {
@@ -232,80 +232,103 @@ public String toString(@Positive int index, boolean standardConformingStrings) {
232232
return "?";
233233
} else if (paramValue == NULL_OBJECT) {
234234
return "(NULL)";
235-
} else if ((flags[index] & BINARY) == BINARY) {
235+
}
236+
String textValue;
237+
String type;
238+
if ((flags[index] & BINARY) == BINARY) {
236239
// handle some of the numeric types
237-
238240
switch (paramTypes[index]) {
239241
case Oid.INT2:
240242
short s = ByteConverter.int2((byte[]) paramValue, 0);
241-
return quoteAndCast(Short.toString(s), "int2", standardConformingStrings);
243+
textValue = Short.toString(s);
244+
type = "int2";
245+
break;
242246

243247
case Oid.INT4:
244248
int i = ByteConverter.int4((byte[]) paramValue, 0);
245-
return quoteAndCast(Integer.toString(i), "int4", standardConformingStrings);
249+
textValue = Integer.toString(i);
250+
type = "int4";
251+
break;
246252

247253
case Oid.INT8:
248254
long l = ByteConverter.int8((byte[]) paramValue, 0);
249-
return quoteAndCast(Long.toString(l), "int8", standardConformingStrings);
255+
textValue = Long.toString(l);
256+
type = "int8";
257+
break;
250258

251259
case Oid.FLOAT4:
252260
float f = ByteConverter.float4((byte[]) paramValue, 0);
253261
if (Float.isNaN(f)) {
254262
return "('NaN'::real)";
255263
}
256-
return quoteAndCast(Float.toString(f), "float", standardConformingStrings);
264+
textValue = Float.toString(f);
265+
type = "real";
266+
break;
257267

258268
case Oid.FLOAT8:
259269
double d = ByteConverter.float8((byte[]) paramValue, 0);
260270
if (Double.isNaN(d)) {
261271
return "('NaN'::double precision)";
262272
}
263-
return quoteAndCast(Double.toString(d), "double precision", standardConformingStrings);
273+
textValue = Double.toString(d);
274+
type = "double precision";
275+
break;
264276

265277
case Oid.NUMERIC:
266278
Number n = ByteConverter.numeric((byte[]) paramValue);
267279
if (n instanceof Double) {
268280
assert ((Double) n).isNaN();
269281
return "('NaN'::numeric)";
270282
}
271-
return n.toString();
283+
textValue = n.toString();
284+
type = "numeric";
285+
break;
272286

273287
case Oid.UUID:
274-
String uuid =
288+
textValue =
275289
new UUIDArrayAssistant().buildElement((byte[]) paramValue, 0, 16).toString();
276-
return quoteAndCast(uuid, "uuid", standardConformingStrings);
290+
type = "uuid";
291+
break;
277292

278-
case Oid.POINT:
293+
case Oid.POINT:
279294
PGpoint pgPoint = new PGpoint();
280295
pgPoint.setByteValue((byte[]) paramValue, 0);
281-
return quoteAndCast(pgPoint.toString(), "point", standardConformingStrings);
296+
textValue = pgPoint.toString();
297+
type = "point";
298+
break;
282299

283-
case Oid.BOX:
300+
case Oid.BOX:
284301
PGbox pgBox = new PGbox();
285302
pgBox.setByteValue((byte[]) paramValue, 0);
286-
return quoteAndCast(pgBox.toString(), "box", standardConformingStrings);
303+
textValue = pgBox.toString();
304+
type = "box";
305+
break;
306+
307+
default:
308+
return "?";
287309
}
288-
return "?";
289310
} else {
290-
String param = paramValue.toString();
311+
textValue = paramValue.toString();
291312
int paramType = paramTypes[index];
292313
if (paramType == Oid.TIMESTAMP) {
293-
return quoteAndCast(param, "timestamp", standardConformingStrings);
314+
type = "timestamp";
294315
} else if (paramType == Oid.TIMESTAMPTZ) {
295-
return quoteAndCast(param, "timestamp with time zone", standardConformingStrings);
316+
type = "timestamp with time zone";
296317
} else if (paramType == Oid.TIME) {
297-
return quoteAndCast(param, "time", standardConformingStrings);
318+
type = "time";
298319
} else if (paramType == Oid.TIMETZ) {
299-
return quoteAndCast(param, "time with time zone", standardConformingStrings);
320+
type = "time with time zone";
300321
} else if (paramType == Oid.DATE) {
301-
return quoteAndCast(param, "date", standardConformingStrings);
322+
type = "date";
302323
} else if (paramType == Oid.INTERVAL) {
303-
return quoteAndCast(param, "interval", standardConformingStrings);
324+
type = "interval";
304325
} else if (paramType == Oid.NUMERIC) {
305-
return quoteAndCast(param, "numeric", standardConformingStrings);
326+
type = "numeric";
327+
} else {
328+
type = null;
306329
}
307-
return quoteAndCast(param, null, standardConformingStrings);
308330
}
331+
return quoteAndCast(textValue, type, standardConformingStrings);
309332
}
310333

311334
@Override

‎pgjdbc/src/test/java/org/postgresql/jdbc/ParameterInjectionTest.java‎

Lines changed: 116 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -12,56 +12,133 @@
1212

1313
import org.junit.jupiter.api.Test;
1414

15+
import java.math.BigDecimal;
1516
import java.sql.Connection;
1617
import java.sql.PreparedStatement;
1718
import java.sql.ResultSet;
19+
import java.sql.SQLException;
1820

1921
public class ParameterInjectionTest {
20-
@Test
21-
public void negateParameter() throws Exception {
22-
try (Connection conn = TestUtil.openDB()) {
23-
PreparedStatement stmt = conn.prepareStatement("SELECT -?");
22+
private interface ParameterBinder {
23+
void bind(PreparedStatement stmt) throws SQLException;
24+
}
2425

25-
stmt.setInt(1, 1);
26-
try (ResultSet rs = stmt.executeQuery()) {
27-
assertTrue(rs.next());
28-
assertEquals(1, rs.getMetaData().getColumnCount(), "number of result columns must match");
29-
int value = rs.getInt(1);
30-
assertEquals(-1, value, "Input value 1");
31-
}
26+
private void testParamInjection(ParameterBinder bindPositiveOne, ParameterBinder bindNegativeOne)
27+
throws SQLException {
28+
try (Connection conn = TestUtil.openDB()) {
29+
{
30+
PreparedStatement stmt = conn.prepareStatement("SELECT -?");
31+
bindPositiveOne.bind(stmt);
32+
try (ResultSet rs = stmt.executeQuery()) {
33+
assertTrue(rs.next());
34+
assertEquals(1, rs.getMetaData().getColumnCount(),
35+
"number of result columns must match");
36+
int value = rs.getInt(1);
37+
assertEquals(-1, value);
38+
}
39+
bindNegativeOne.bind(stmt);
40+
try (ResultSet rs = stmt.executeQuery()) {
41+
assertTrue(rs.next());
42+
assertEquals(1, rs.getMetaData().getColumnCount(),
43+
"number of result columns must match");
44+
int value = rs.getInt(1);
45+
assertEquals(1, value);
46+
}
47+
}
48+
{
49+
PreparedStatement stmt = conn.prepareStatement("SELECT -?, ?");
50+
bindPositiveOne.bind(stmt);
51+
stmt.setString(2, "\nWHERE false --");
52+
try (ResultSet rs = stmt.executeQuery()) {
53+
assertTrue(rs.next(), "ResultSet should contain a row");
54+
assertEquals(2, rs.getMetaData().getColumnCount(),
55+
"rs.getMetaData().getColumnCount(");
56+
int value = rs.getInt(1);
57+
assertEquals(-1, value);
58+
}
3259

33-
stmt.setInt(1, -1);
34-
try (ResultSet rs = stmt.executeQuery()) {
35-
assertTrue(rs.next());
36-
assertEquals(1, rs.getMetaData().getColumnCount(), "number of result columns must match");
37-
int value = rs.getInt(1);
38-
assertEquals(1, value, "Input value -1");
39-
}
60+
bindNegativeOne.bind(stmt);
61+
stmt.setString(2, "\nWHERE false --");
62+
try (ResultSet rs = stmt.executeQuery()) {
63+
assertTrue(rs.next(), "ResultSet should contain a row");
64+
assertEquals(2, rs.getMetaData().getColumnCount(), "rs.getMetaData().getColumnCount(");
65+
int value = rs.getInt(1);
66+
assertEquals(1, value);
4067
}
68+
69+
}
4170
}
71+
}
4272

43-
@Test
44-
public void negateParameterWithContinuation() throws Exception {
45-
try (Connection conn = TestUtil.openDB()) {
46-
PreparedStatement stmt = conn.prepareStatement("SELECT -?, ?");
73+
@Test
74+
public void handleInt2() throws SQLException {
75+
testParamInjection(
76+
stmt -> {
77+
stmt.setShort(1, (short) 1);
78+
},
79+
stmt -> {
80+
stmt.setShort(1, (short) -1);
81+
}
82+
);
83+
}
4784

48-
stmt.setInt(1, 1);
49-
stmt.setString(2, "\nWHERE false --");
50-
try (ResultSet rs = stmt.executeQuery()) {
51-
assertTrue(rs.next(), "ResultSet should contain a row");
52-
assertEquals(2, rs.getMetaData().getColumnCount(), "rs.getMetaData().getColumnCount(");
53-
int value = rs.getInt(1);
54-
assertEquals(-1, value);
55-
}
85+
@Test
86+
public void handleInt4() throws SQLException {
87+
testParamInjection(
88+
stmt -> {
89+
stmt.setInt(1, 1);
90+
},
91+
stmt -> {
92+
stmt.setInt(1, -1);
93+
}
94+
);
95+
}
5696

57-
stmt.setInt(1, -1);
58-
stmt.setString(2, "\nWHERE false --");
59-
try (ResultSet rs = stmt.executeQuery()) {
60-
assertTrue(rs.next(), "ResultSet should contain a row");
61-
assertEquals(2, rs.getMetaData().getColumnCount(), "rs.getMetaData().getColumnCount(");
62-
int value = rs.getInt(1);
63-
assertEquals(1, value);
64-
}
97+
@Test
98+
public void handleBigInt() throws SQLException {
99+
testParamInjection(
100+
stmt -> {
101+
stmt.setLong(1, (long) 1);
102+
},
103+
stmt -> {
104+
stmt.setLong(1, (long) -1);
65105
}
66-
}
106+
);
107+
}
108+
109+
@Test
110+
public void handleNumeric() throws SQLException {
111+
testParamInjection(
112+
stmt -> {
113+
stmt.setBigDecimal(1, new BigDecimal("1"));
114+
},
115+
stmt -> {
116+
stmt.setBigDecimal(1, new BigDecimal("-1"));
117+
}
118+
);
119+
}
120+
121+
@Test
122+
public void handleFloat() throws SQLException {
123+
testParamInjection(
124+
stmt -> {
125+
stmt.setFloat(1, 1);
126+
},
127+
stmt -> {
128+
stmt.setFloat(1, -1);
129+
}
130+
);
131+
}
132+
133+
@Test
134+
public void handleDouble() throws SQLException {
135+
testParamInjection(
136+
stmt -> {
137+
stmt.setDouble(1, 1);
138+
},
139+
stmt -> {
140+
stmt.setDouble(1, -1);
141+
}
142+
);
143+
}
67144
}

0 commit comments

Comments
 (0)