GHSA-CGFV-JRFP-2R7V: Root-Cause Fix for OpenRemote Crosstab SQL Injection

Vulnerability

GHSA-CGFV-JRFP-2R7V describes an authenticated SQL injection in OpenRemote's datapoint crosstab CSV export. The vulnerable path built PostgreSQL crosstab(...) queries using header strings derived from asset names and attribute names. In the pre-patch implementation, display headers were assembled from assetStorageService.findNames(...) and attribute names, then interpolated into both the category query and the crosstab column definition list.

This is dangerous because asset names are tenant-controlled data. Even though the old code doubled single quotes for the VALUES category list, it also used the same header strings as SQL identifiers via quoted column definitions such as "header" text. That left SQL syntax exposed to delimiter and identifier-breaking input, especially in the crosstab-specific query construction. The advisory impact is consistent with data exfiltration risk from a multi-tenant PostgreSQL backend through crafted asset names during export.

// vulnerable pattern from the commit diff summary
Set<String> headers = getAttributeHeaders(attributeRefs);
String categoryValues = headers.stream()
    .map(header -> "('" + header.replace("'", "''") + "')")
    .collect(Collectors.joining(", "));
String categoryQuery = "SELECT header FROM (VALUES " + categoryValues + ") AS t(header)";
String attributeColumns = headers.stream()
    .map(header -> "\"" + header + "\" text")
    .collect(Collectors.joining(", "));

The core flaw is not merely missing escaping in one branch; it is the design choice of treating untrusted display labels as SQL structure. That is the root cause the patch needs to eliminate. See the fixing commit 02ac83074b81617add814b2a72d459abdf374147 and the advisory GHSA-CGFV-JRFP-2R7V.

Patch

The patch restructures crosstab export generation so that SQL only sees generated, internal column keys rather than user-controlled labels. It introduces an ExportColumn model carrying an ordinal, the original AttributeRef, a display header, and a generated columnKey like col_0. These values are inserted into a temporary table with explicit columns (ordinal, entity_id, attribute_name, column_key).

For crosstab exports, the category query now reads ordered column_key values from the temp table, and the crosstab result columns are defined from those generated keys only. Human-readable labels are no longer embedded in SQL. Instead, the service writes a custom CSV header line using buildCrosstabCsvHeader(...) and escapeCsvField(...), while PostgreSQL COPY is invoked without automatic CSV header generation for crosstab output.

// patched pattern from the commit diff summary
String categoryQuery = "select column_key from " + tempTableName + " order by ordinal";
String attributeColumns = exportColumns.stream()
    .map(exportColumn -> exportColumn.columnKey() + " text")
    .collect(Collectors.joining(", "));

if (isCrosstabFormat(format)) {
    out.write(buildCrosstabCsvHeader(exportColumns).getBytes(StandardCharsets.UTF_8));
}

The patch also adds a regression test covering an asset name containing SQL delimiter syntax: SQL "delimiter" $cat$ -- label. The test verifies both crosstab formats export the datapoint correctly and preserve the malicious-looking string only as a CSV label, not as executable SQL. This is strong evidence that the fix specifically targets the exploit primitive described in the advisory. Source: commit diff.

Review

Pros

• Eliminates the root unsafe pattern by separating SQL identifiers/categories from display labels. Generated column_key values are deterministic internal tokens, so attacker-controlled asset names no longer participate in SQL syntax.
• Preserves output semantics for users by emitting the original display labels in the CSV header, now handled as data with explicit CSV escaping rather than SQL structure.
• Adds a targeted regression test using delimiter-heavy input including quotes and $cat$, which is directly relevant to the prior crosstab query construction.
• Improves ordering determinism through explicit ordinal assignment, avoiding dependence on sorted label strings inside SQL generation.
• Reduces reliance on ad hoc escaping. The old implementation attempted to escape single quotes for one SQL context but still reused the same strings in another SQL context; the new design avoids that class of mistake.

Cons

• The patch summary shows temp table names are still concatenated into SQL strings. These names appear internally generated rather than user-controlled, but the safety of that assumption depends on their construction elsewhere in the code.
• Attribute names are still inserted into the temp table and used in joins as data. That is appropriate here, but broader review should confirm no other export path later reuses them as SQL syntax.
• The new manual CSV header path introduces custom CSV escaping logic. The included helper appears correct for commas, quotes, CR, and LF in the shown snippet, but custom serializers are generally more maintenance-sensitive than library-backed encoding.
• The added test is focused on crosstab formats and asset-name payloads. It does not demonstrate coverage for duplicate display labels, unusual Unicode, or very large attribute sets, though those are correctness concerns more than direct security gaps.

Verdict

Root-cause.

This patch addresses the vulnerability at the correct abstraction boundary. The original issue was caused by embedding untrusted display text into SQL category literals and column definitions for PostgreSQL crosstab export. The fix removes display text from SQL entirely, replacing it with generated column keys and moving user-visible labels into a separately escaped CSV header. That is a structural remediation rather than a narrow escaping tweak.

Based on the commit 02ac83074b81617add814b2a72d459abdf374147 and the advisory GHSA-CGFV-JRFP-2R7V, the patch is well-aligned with the reported exploit path and should materially close the authenticated SQL injection vector in crosstab export.