Skip to content

Commit 5633f15

Browse files
wyattwalterclaude
andcommitted
fix(interfaces): validate WHERE clause column names in UQI filtering
Check every column referenced in a WHERE condition, including nested AND/OR groups, against the columns present in the data before building the filter query, and escape quote characters in column identifiers. GHSA: https://github.com/appsmithorg/appsmith/security/advisories/GHSA-cm7r-f7h3-q33p Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent fc8916f commit 5633f15

2 files changed

Lines changed: 93 additions & 2 deletions

File tree

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/services/ce/FilterDataServiceCE.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ public ArrayNode filterDataNew(
144144

145145
validateProjectionColumns(uqiDataFilterParams.getProjectionColumns(), schema);
146146
validateSortByColumns(uqiDataFilterParams.getSortBy(), schema);
147+
validateConditionColumns(uqiDataFilterParams.getCondition(), schema);
147148

148149
String tableName = generateTable(schema);
149150
try {
@@ -867,6 +868,30 @@ public boolean validConditionList(List<Condition> conditionList, Map<String, Dat
867868
return true;
868869
}
869870

871+
public void validateConditionColumns(Condition condition, Map<String, DataType> schema) {
872+
if (condition == null) {
873+
return;
874+
}
875+
876+
ConditionalOperator operator = condition.getOperator();
877+
if (operator == ConditionalOperator.AND || operator == ConditionalOperator.OR) {
878+
Object value = condition.getValue();
879+
if (value instanceof List) {
880+
List<Condition> childConditions = (List<Condition>) value;
881+
for (Condition child : childConditions) {
882+
validateConditionColumns(child, schema);
883+
}
884+
}
885+
} else {
886+
String path = condition.getPath();
887+
if (StringUtils.isNotEmpty(path) && !schema.containsKey(path)) {
888+
throw new AppsmithPluginException(
889+
AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR,
890+
path + " not found in the known column names :" + schema.keySet());
891+
}
892+
}
893+
}
894+
870895
public String generateLogicalExpression(
871896
List<Condition> conditions,
872897
List<PreparedStatementValueDTO> values,
@@ -898,9 +923,10 @@ public String generateLogicalExpression(
898923
sb.append(" " + logicOp);
899924
}
900925
if (StringUtils.isNotEmpty(path)) {
926+
String escapedPath = path.replace("\"", "\"\"");
901927
if (value == null || value.equals(StringUtils.EMPTY)) {
902928
sb.append(" ( ");
903-
sb.append("\"" + path + "\"");
929+
sb.append("\"" + escapedPath + "\"");
904930
sb.append(" ");
905931
if (Set.of(
906932
ConditionalOperator.EQ,
@@ -926,7 +952,7 @@ public String generateLogicalExpression(
926952
operator + " is not supported currently for filtering.");
927953
}
928954
sb.append(" ( ");
929-
sb.append("\"" + path + "\"");
955+
sb.append("\"" + escapedPath + "\"");
930956
sb.append(" ");
931957
sb.append(sqlOp);
932958
sb.append(" ");

app/server/appsmith-interfaces/src/test/java/com/appsmith/external/services/FilterDataServiceTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.appsmith.external.constants.ConditionalOperator;
44
import com.appsmith.external.constants.DataType;
5+
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
56
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
67
import com.appsmith.external.models.Condition;
78
import com.appsmith.external.models.UQIDataFilterParams;
@@ -1504,4 +1505,68 @@ public void testInvalidProjectionDoesNotLeakTable() throws IOException {
15041505
filterDataService.filterDataNew(items, new UQIDataFilterParams(null, List.of("id"), null, null));
15051506
assertEquals(2, result.size());
15061507
}
1508+
1509+
private Condition whereClause(Map<String, Object>... children) {
1510+
return parseWhereClause(Map.of("condition", "AND", "children", List.of(children)));
1511+
}
1512+
1513+
private void assertUnknownColumnRejected(Condition condition, String columnName) throws IOException {
1514+
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);
1515+
1516+
AppsmithPluginException exception = assertThrows(
1517+
AppsmithPluginException.class,
1518+
() -> filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null)));
1519+
1520+
assertEquals(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, exception.getError());
1521+
assertThat(exception.getMessage()).startsWith(columnName + " not found in the known column names");
1522+
}
1523+
1524+
@Test
1525+
public void testWhereWithUnknownColumn_throwsException() throws IOException {
1526+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "unknownColumn", "value", "1"));
1527+
1528+
assertUnknownColumnRejected(condition, "unknownColumn");
1529+
}
1530+
1531+
@Test
1532+
public void testWhereWithUnknownColumnInNestedGroup_throwsException() throws IOException {
1533+
Condition condition = whereClause(
1534+
Map.of("condition", "EQ", "key", "id", "value", "1"),
1535+
Map.of(
1536+
"condition",
1537+
"OR",
1538+
"children",
1539+
List.of(Map.of("condition", "EQ", "key", "unknownColumn", "value", ""))));
1540+
1541+
assertUnknownColumnRejected(condition, "unknownColumn");
1542+
}
1543+
1544+
@Test
1545+
public void testWhereWithColumnNameContainingQuote_throwsException() throws IOException {
1546+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", ""));
1547+
1548+
assertUnknownColumnRejected(condition, "na\"me");
1549+
}
1550+
1551+
@Test
1552+
public void testWhereWithKnownColumn_filtersRows() throws IOException {
1553+
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);
1554+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "id", "value", "1"));
1555+
1556+
ArrayNode result = filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null));
1557+
1558+
assertEquals(1, result.size());
1559+
assertEquals(1, result.get(0).get("id").asInt());
1560+
}
1561+
1562+
@Test
1563+
public void testGenerateLogicalExpression_quotesColumnNameContainingQuote() {
1564+
Map<String, DataType> schema = Map.of("na\"me", DataType.STRING);
1565+
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", "Alice"));
1566+
1567+
String expression = filterDataService.generateLogicalExpression(
1568+
(List<Condition>) condition.getValue(), new ArrayList<>(), schema, condition.getOperator());
1569+
1570+
assertThat(expression).isEqualTo(" ( \"na\"\"me\" = ? ) ");
1571+
}
15071572
}

0 commit comments

Comments
 (0)