Skip to content

Commit ea49233

Browse files
authored
MINOR: Modify logic to use parameterized queries (#24902)
1 parent 2a78ba5 commit ea49233

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ private String getEventSubscriptionAlertType() {
153153
return "";
154154
} else {
155155
if (Boolean.TRUE.equals(DatasourceConfig.getInstance().isMySQL())) {
156-
return String.format("JSON_EXTRACT(json, '$.alertType') = '%s'", alertType);
156+
return "JSON_UNQUOTE(JSON_EXTRACT(json, '$.alertType')) = :alertType";
157157
} else {
158-
return String.format("json->>'alertType' = '%s'", alertType);
158+
return "json->>'alertType' = :alertType";
159159
}
160160
}
161161
}
@@ -166,10 +166,9 @@ private String getNotificationTemplateCondition() {
166166
return "";
167167
} else {
168168
if (Boolean.TRUE.equals(DatasourceConfig.getInstance().isMySQL())) {
169-
return String.format(
170-
"JSON_EXTRACT(json, '$.notificationTemplate.id') = '%s'", notificationTemplate);
169+
return "JSON_UNQUOTE(JSON_EXTRACT(json, '$.notificationTemplate.id')) = :notificationTemplate";
171170
} else {
172-
return String.format("json->'notificationTemplate'->>'id' = '%s'", notificationTemplate);
171+
return "json->'notificationTemplate'->>'id' = :notificationTemplate";
173172
}
174173
}
175174
}

openmetadata-service/src/test/java/org/openmetadata/service/resources/events/EventSubscriptionResourceTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,4 +2598,46 @@ void test_querySubscriptionsByTemplate(TestInfo test) throws IOException {
25982598
templateTest.deleteEntity(template1.getId(), ADMIN_AUTH_HEADERS);
25992599
templateTest.deleteEntity(template2.getId(), ADMIN_AUTH_HEADERS);
26002600
}
2601+
2602+
private void assertSqlInjectionAttempt(Map<String, String> params) throws IOException {
2603+
ResultList<EventSubscription> results = null;
2604+
try {
2605+
results = listEntities(params, ADMIN_AUTH_HEADERS);
2606+
} catch (HttpResponseException e) {
2607+
// SQL injection should not crash the server with a 500 error
2608+
assertTrue(e.getStatusCode() != 500, "SQL injection should not cause internal server error");
2609+
return;
2610+
}
2611+
2612+
// If no exception is thrown, malicious input must not expose any data
2613+
assertNotNull(results, "Malicious input should not produce null results");
2614+
assertTrue(
2615+
results.getData() == null || results.getData().isEmpty(),
2616+
"Malicious input should return empty results");
2617+
}
2618+
2619+
@Test
2620+
void test_parameterizedQueriesPreventSqlInjection(TestInfo test) throws IOException {
2621+
Map<String, String> sqlInjectionAttempts = new HashMap<>();
2622+
sqlInjectionAttempts.put("alertType", "Observability' OR '1'='1");
2623+
assertSqlInjectionAttempt(sqlInjectionAttempts);
2624+
2625+
sqlInjectionAttempts.clear();
2626+
sqlInjectionAttempts.put("alertType", "Observability'--");
2627+
assertSqlInjectionAttempt(sqlInjectionAttempts);
2628+
2629+
sqlInjectionAttempts.clear();
2630+
sqlInjectionAttempts.put("alertType", "' UNION SELECT * FROM user_entity--");
2631+
assertSqlInjectionAttempt(sqlInjectionAttempts);
2632+
2633+
sqlInjectionAttempts.clear();
2634+
sqlInjectionAttempts.put(
2635+
"notificationTemplate", "00000000-0000-0000-0000-000000000000' OR '1'='1");
2636+
assertSqlInjectionAttempt(sqlInjectionAttempts);
2637+
2638+
Map<String, String> validParams = new HashMap<>();
2639+
validParams.put("alertType", "Observability");
2640+
ResultList<EventSubscription> validResults = listEntities(validParams, ADMIN_AUTH_HEADERS);
2641+
assertNotNull(validResults, "Valid alertType should work correctly");
2642+
}
26012643
}

0 commit comments

Comments
 (0)