From 0d2afe2458c9cb1928f1af3bae4474d54f1a4069 Mon Sep 17 00:00:00 2001 From: Clemens Portele Date: Thu, 2 Jul 2026 09:17:44 +0200 Subject: [PATCH] fix SQL injection through mutation values on the write path Mutation SQL is assembled by string concatenation, and numeric-typed columns inlined the value's raw text: SqlMutationSession.encodeLiteral returned value.asText() for INTEGER/FLOAT, and FeatureEncoderSql.onValue only quoted STRING/DATE/DATETIME. A client sending a JSON string (or a wfs:Value) for a numeric column could therefore inject arbitrary SQL into the generated INSERT/UPDATE (e.g. "0 WHERE 1=1; DROP TABLE x --"), without requiring handling=strict. FeatureEncoderSql.onValue also inlined BOOLEAN values raw, and the role-override UPDATE path (formatRoleOverrideValue) had the same shape. Centralize inline-literal rendering in SqlLiterals and route all three sinks through it: - integers are re-rendered from a parsed BigDecimal.toBigIntegerExact() - floats are re-rendered via BigDecimal.toPlainString() (also avoids non-portable scientific notation) - booleans are normalized to the TRUE/FALSE keywords - strings/dates and any unmapped column type are single-quoted with quote-doubling (safe default: never inline raw input) - null is preserved A value that does not match its column type is now rejected with IllegalArgumentException instead of being inlined verbatim, consistent with the existing geometry-literal validation. This is a slightly stricter contract: a malformed numeric/boolean value now fails the write rather than being silently coerced. Oracle shares this path (FeatureProviderOracle extends FeatureProviderSql and reuses SqlMutationSession/FeatureEncoderSql; SqlDialectOras.escapeString matches the standard quote-doubling), so the guard applies there too. Adds SqlLiteralsSpec covering quote-doubling, numeric/boolean validation, injection rejection, NULL, and the quoted fallback. --- .../features/sql/app/FeatureEncoderSql.java | 16 +--- .../features/sql/app/SqlLiterals.java | 92 ++++++++++++++++++ .../features/sql/app/SqlMutationSession.java | 27 +----- .../features/sql/app/SqlLiteralsSpec.groovy | 94 +++++++++++++++++++ 4 files changed, 195 insertions(+), 34 deletions(-) create mode 100644 xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java create mode 100644 xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlLiteralsSpec.groovy diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java index dd3980107..ef6d472d8 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java @@ -13,7 +13,6 @@ import de.ii.xtraplatform.crs.domain.EpsgCrs; import de.ii.xtraplatform.features.domain.MappingRule; import de.ii.xtraplatform.features.domain.MappingRule.Scope; -import de.ii.xtraplatform.features.domain.SchemaBase; import de.ii.xtraplatform.features.domain.SchemaBase.Type; import de.ii.xtraplatform.features.domain.pipeline.FeatureEventHandlerSimple.ModifiableContext; import de.ii.xtraplatform.features.domain.pipeline.FeatureTokenEncoderBaseSimple; @@ -313,17 +312,12 @@ public void onValue(ModifiableContext context) return; } - boolean needsQuotes = - column.second().getType() == SchemaBase.Type.STRING - || column.second().getType() == SchemaBase.Type.DATETIME - || column.second().getType() == SchemaBase.Type.DATE; - /* (schemaSql.getType() == SchemaBase.Type.FEATURE_REF - && schemaSql.getValueType().orElse(SchemaBase.Type.STRING) - == SchemaBase.Type.STRING)*/ - + // Numeric/boolean values are validated and re-rendered (never inlined as raw request + // text); string/date values are quoted with quote-doubling. A null stays null so the + // downstream row renderer emits SQL NULL. See SqlLiterals. value = - needsQuotes && Objects.nonNull(value) - ? String.format("'%s'", value.replaceAll("'", "''")) + Objects.nonNull(value) + ? SqlLiterals.forType(column.second().getType(), value) : value; boolean junctionElement = diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java new file mode 100644 index 000000000..c98a74466 --- /dev/null +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java @@ -0,0 +1,92 @@ +/* + * Copyright 2025 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.sql.app; + +import de.ii.xtraplatform.features.domain.SchemaBase; +import java.math.BigDecimal; +import java.util.Locale; + +/** + * Renders a typed value as an inline SQL literal for the mutation path. + * + *

The mutation SQL is assembled by string concatenation rather than JDBC bind parameters, so + * every value inlined here must be provably safe: string/date literals are single-quoted with + * quote-doubling, numeric literals are re-rendered from a parsed {@link BigDecimal} (never the raw + * input text), and booleans are normalized to the {@code TRUE}/{@code FALSE} keywords. A value that + * does not match its column type is rejected with {@link IllegalArgumentException} instead of being + * inlined verbatim. + */ +final class SqlLiterals { + + private SqlLiterals() {} + + static String forType(SchemaBase.Type type, String value) { + if (value == null) { + return "NULL"; + } + switch (type) { + case INTEGER: + return integer(value); + case FLOAT: + return number(value); + case BOOLEAN: + return bool(value); + case STRING: + case DATE: + case DATETIME: + default: + // Safe default: quote everything not explicitly typed as numeric/boolean. Quoting an + // unexpected column type yields a literal PostgreSQL/Oracle will coerce, and never lets + // raw input reach the statement unescaped. + return string(value); + } + } + + static String string(String value) { + if (value == null) { + return "NULL"; + } + return "'" + value.replace("'", "''") + "'"; + } + + static String integer(String value) { + try { + return new BigDecimal(value.trim()).toBigIntegerExact().toString(); + } catch (NumberFormatException | ArithmeticException e) { + throw new IllegalArgumentException("not a valid integer value: '" + value + "'"); + } + } + + static String number(String value) { + try { + // toPlainString avoids scientific notation (e.g. "1E+3"), which is not portable across all + // SQL dialects. + return new BigDecimal(value.trim()).toPlainString(); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("not a valid number value: '" + value + "'"); + } + } + + static String bool(String value) { + String normalized = value.trim().toLowerCase(Locale.ROOT); + switch (normalized) { + case "true": + case "t": + case "1": + case "yes": + return "TRUE"; + case "false": + case "f": + case "0": + case "no": + return "FALSE"; + default: + throw new IllegalArgumentException("not a valid boolean value: '" + value + "'"); + } + } +} diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java index 0729a9697..5c20c2e91 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java @@ -279,14 +279,7 @@ private static void applyRoleOverrides( } private static String formatRoleOverrideValue(SqlQueryColumn column, Object value) { - SchemaBase.Type type = column.getType(); - String raw = value.toString(); - if (type == SchemaBase.Type.STRING - || type == SchemaBase.Type.DATETIME - || type == SchemaBase.Type.DATE) { - return "'" + raw.replace("'", "''") + "'"; - } - return raw; + return SqlLiterals.forType(column.getType(), value.toString()); } @Override @@ -1422,21 +1415,9 @@ private String encodeLiteral( || column.hasOperation(SqlQueryColumn.Operation.WKB)) { return encodeGeometryLiteral(column, value, crs); } - SchemaBase.Type type = column.getType(); - if (type == SchemaBase.Type.STRING - || type == SchemaBase.Type.DATETIME - || type == SchemaBase.Type.DATE) { - return sqlString(value.asText()); - } - if (type == SchemaBase.Type.BOOLEAN) { - return value.asBoolean() ? "TRUE" : "FALSE"; - } - if (type == SchemaBase.Type.INTEGER || type == SchemaBase.Type.FLOAT) { - return value.asText(); - } - // Fallback — treat as string. Avoids generating invalid SQL on niche column types we haven't - // wired up explicitly yet (FEATURE_REF, etc.). - return sqlString(value.asText()); + // Numeric/boolean values are validated and re-rendered (never inlined as raw request text); + // everything else is quoted. See SqlLiterals. + return SqlLiterals.forType(column.getType(), value.asText()); } private String encodeGeometryLiteral( diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlLiteralsSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlLiteralsSpec.groovy new file mode 100644 index 000000000..f469e1bd9 --- /dev/null +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlLiteralsSpec.groovy @@ -0,0 +1,94 @@ +/* + * Copyright 2025 interactive instruments GmbH + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package de.ii.xtraplatform.features.sql.app + +import de.ii.xtraplatform.features.domain.SchemaBase +import spock.lang.Specification + +class SqlLiteralsSpec extends Specification { + + def "string values are single-quoted with quote-doubling"() { + expect: + SqlLiterals.forType(SchemaBase.Type.STRING, value) == expected + + where: + value || expected + "abc" || "'abc'" + "O'Brien" || "'O''Brien'" + "'; DROP TABLE x--" || "'''; DROP TABLE x--'" + "" || "''" + } + + def "integer values are re-rendered from a parsed number"() { + expect: + SqlLiterals.forType(SchemaBase.Type.INTEGER, value) == expected + + where: + value || expected + "42" || "42" + "-7" || "-7" + "+7" || "7" + " 42 " || "42" + "42.0" || "42" + "1e3" || "1000" + } + + def "float values are re-rendered without scientific notation"() { + expect: + SqlLiterals.forType(SchemaBase.Type.FLOAT, value) == expected + + where: + value || expected + "42" || "42" + "3.14" || "3.14" + "-0.5" || "-0.5" + "1.5e-3" || "0.0015" + } + + def "boolean values are normalized to the SQL keywords"() { + expect: + SqlLiterals.forType(SchemaBase.Type.BOOLEAN, value) == expected + + where: + value || expected + "true" || "TRUE" + "TRUE" || "TRUE" + "1" || "TRUE" + "false" || "FALSE" + "0" || "FALSE" + } + + def "injection attempts through a numeric column are rejected, not inlined"() { + when: + SqlLiterals.forType(type, value) + + then: + thrown(IllegalArgumentException) + + where: + type | value + SchemaBase.Type.INTEGER | "0 WHERE 1=1; DROP TABLE x --" + SchemaBase.Type.INTEGER | "42; DELETE FROM t" + SchemaBase.Type.INTEGER | "42.5" + SchemaBase.Type.FLOAT | "3.14 OR 1=1" + SchemaBase.Type.FLOAT | "NaN); DROP" + SchemaBase.Type.BOOLEAN | "true; DROP TABLE x" + SchemaBase.Type.INTEGER | "" + } + + def "null values become the SQL NULL keyword"() { + expect: + SqlLiterals.forType(SchemaBase.Type.INTEGER, null) == "NULL" + SqlLiterals.forType(SchemaBase.Type.STRING, null) == "NULL" + } + + def "unmapped column types fall back to a quoted literal rather than raw inlining"() { + expect: + SqlLiterals.forType(SchemaBase.Type.FEATURE_REF, "abc'; DROP") == "'abc''; DROP'" + } +}