Skip to content

Commit 643fc45

Browse files
phrockerphrocker
committed
Address risks
1 parent 732453c commit 643fc45

File tree

1 file changed

+82
-75
lines changed

1 file changed

+82
-75
lines changed

‎integration-proxy/src/main/java/io/sentrius/sso/controllers/api/DatabaseProxyController.java‎

Lines changed: 82 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,7 @@ public ResponseEntity<?> executeQuery(
8080
return ResponseEntity.status(HttpStatus.SC_UNAUTHORIZED).body("User not authenticated");
8181
}
8282

83-
List<IntegrationSecurityToken> databaseIntegrations = integrationSecurityTokenService
84-
.findByConnectionType("database");
85-
86-
if (databaseIntegrations.isEmpty()) {
87-
return ResponseEntity.status(HttpStatus.SC_NOT_FOUND).body("No database integration configured");
88-
}
89-
90-
IntegrationSecurityToken databaseIntegration = databaseIntegrations.get(0);
91-
ExternalIntegrationDTO integrationDTO = new ExternalIntegrationDTO(databaseIntegration, true);
92-
83+
// Validate query input first before checking database integration
9384
String query = (String) queryPayload.get("query");
9485
if (query == null || query.trim().isEmpty()) {
9586
return ResponseEntity.badRequest().body(Map.of("error", "Query parameter is required"));
@@ -100,39 +91,54 @@ public ResponseEntity<?> executeQuery(
10091
.body(Map.of("error", "Only SELECT queries are allowed"));
10192
}
10293

103-
if (!isSafeSelectQuery(query)) {
104-
return ResponseEntity.status(HttpStatus.SC_FORBIDDEN)
105-
.body(Map.of("error", "Query is not allowed"));
94+
List<IntegrationSecurityToken> databaseIntegrations = integrationSecurityTokenService
95+
.findByConnectionType("database");
96+
97+
if (databaseIntegrations.isEmpty()) {
98+
return ResponseEntity.status(HttpStatus.SC_NOT_FOUND).body("No database integration configured");
10699
}
107100

101+
IntegrationSecurityToken databaseIntegration = databaseIntegrations.get(0);
102+
ExternalIntegrationDTO integrationDTO = new ExternalIntegrationDTO(databaseIntegration, true);
103+
108104
String jdbcUrl = buildJdbcUrl(integrationDTO);
109105
List<Map<String, Object>> results = new ArrayList<>();
110106

107+
// Set a maximum row limit to prevent resource exhaustion
108+
final int MAX_ROWS = 1000;
109+
111110
try (Connection conn = DriverManager.getConnection(
112111
jdbcUrl,
113112
integrationDTO.getUsername(),
114113
integrationDTO.getApiToken()
115-
);
116-
Statement stmt = conn.createStatement();
117-
ResultSet rs = stmt.executeQuery(query)) {
118-
119-
ResultSetMetaData metaData = rs.getMetaData();
120-
int columnCount = metaData.getColumnCount();
121-
122-
while (rs.next()) {
123-
Map<String, Object> row = new LinkedHashMap<>();
124-
for (int i = 1; i <= columnCount; i++) {
125-
row.put(metaData.getColumnName(i), rs.getObject(i));
114+
)) {
115+
// Set connection to read-only mode to prevent any data modification
116+
conn.setReadOnly(true);
117+
118+
try (Statement stmt = conn.createStatement()) {
119+
// Set max rows to prevent excessive memory usage
120+
stmt.setMaxRows(MAX_ROWS);
121+
122+
try (ResultSet rs = stmt.executeQuery(query)) {
123+
ResultSetMetaData metaData = rs.getMetaData();
124+
int columnCount = metaData.getColumnCount();
125+
126+
while (rs.next()) {
127+
Map<String, Object> row = new LinkedHashMap<>();
128+
for (int i = 1; i <= columnCount; i++) {
129+
row.put(metaData.getColumnName(i), rs.getObject(i));
130+
}
131+
results.add(row);
132+
}
133+
134+
return ResponseEntity.ok(Map.of(
135+
"success", true,
136+
"rowCount", results.size(),
137+
"data", results,
138+
"maxRows", MAX_ROWS
139+
));
126140
}
127-
results.add(row);
128141
}
129-
130-
return ResponseEntity.ok(Map.of(
131-
"success", true,
132-
"rowCount", results.size(),
133-
"data", results
134-
));
135-
136142
} catch (SQLException e) {
137143
log.error("Database query failed", e);
138144
return ResponseEntity.status(HttpStatus.SC_INTERNAL_SERVER_ERROR)
@@ -148,48 +154,6 @@ public ResponseEntity<?> executeQuery(
148154
}
149155
}
150156

151-
/**
152-
* Performs basic validation to ensure that only reasonably safe SELECT queries are executed.
153-
* This method is intentionally conservative and will reject queries containing
154-
* multiple statements, comments, or obvious DDL/DML keywords.
155-
*/
156-
private boolean isSafeSelectQuery(String query) {
157-
if (query == null) {
158-
return false;
159-
}
160-
161-
String trimmed = query.trim();
162-
if (trimmed.isEmpty()) {
163-
return false;
164-
}
165-
166-
// Must start with SELECT (case-insensitive)
167-
String upper = trimmed.toUpperCase(Locale.ROOT);
168-
if (!upper.startsWith("SELECT")) {
169-
return false;
170-
}
171-
172-
// Disallow multiple statements and common comment syntaxes
173-
if (upper.contains(";") || upper.contains("--") || upper.contains("/*") || upper.contains("*/") || upper.contains("#")) {
174-
return false;
175-
}
176-
177-
// Disallow obviously dangerous keywords that should not appear in a simple read-only query
178-
String[] forbiddenKeywords = {
179-
" INSERT ", " UPDATE ", " DELETE ", " DROP ", " ALTER ", " TRUNCATE ",
180-
" CREATE ", " MERGE ", " GRANT ", " REVOKE ", " EXEC ", " EXECUTE ",
181-
" INTO OUTFILE ", " INTO DUMPFILE "
182-
};
183-
184-
for (String keyword : forbiddenKeywords) {
185-
if (upper.contains(keyword)) {
186-
return false;
187-
}
188-
}
189-
190-
return true;
191-
}
192-
193157
@GetMapping("/tables")
194158
@Endpoint(description = "List database tables")
195159
@LimitAccess(applicationAccess = {ApplicationAccessEnum.CAN_LOG_IN})
@@ -282,6 +246,13 @@ public ResponseEntity<?> getTableSchema(
282246
return ResponseEntity.status(HttpStatus.SC_UNAUTHORIZED).body("User not authenticated");
283247
}
284248

249+
// Validate table name to prevent SQL injection
250+
if (!isValidTableName(tableName)) {
251+
log.warn("Invalid table name format: {}", tableName);
252+
return ResponseEntity.badRequest()
253+
.body(Map.of("error", "Invalid table name format. Only alphanumeric characters, underscores, and dots are allowed."));
254+
}
255+
285256
List<IntegrationSecurityToken> databaseIntegrations = integrationSecurityTokenService
286257
.findByConnectionType("database");
287258

@@ -350,6 +321,42 @@ private String buildJdbcUrl(ExternalIntegrationDTO integrationDTO) {
350321

351322
private boolean isSelectQuery(String query) {
352323
String trimmedQuery = query.trim().toLowerCase();
353-
return trimmedQuery.startsWith("select");
324+
325+
// Remove SQL comments to prevent bypass attempts
326+
String cleanedQuery = trimmedQuery
327+
.replaceAll("--[^\r\n]*", "") // Remove single-line comments (handles both \n and \r\n)
328+
.replaceAll("(?s)/\\*.*?\\*/", "") // Remove multi-line comments (DOTALL flag for newlines)
329+
.replaceAll("\\s+", " ") // Normalize whitespace
330+
.trim();
331+
332+
// Check if query starts with SELECT
333+
if (!cleanedQuery.startsWith("select")) {
334+
return false;
335+
}
336+
337+
// Block dangerous keywords that could be used for SQL injection
338+
String[] dangerousKeywords = {
339+
";", "exec", "execute", "drop", "delete", "insert", "update",
340+
"create", "alter", "truncate", "grant", "revoke", "xp_"
341+
};
342+
343+
for (String keyword : dangerousKeywords) {
344+
if (cleanedQuery.contains(keyword)) {
345+
log.warn("Blocked query containing dangerous keyword: {}", keyword);
346+
return false;
347+
}
348+
}
349+
350+
return true;
351+
}
352+
353+
private boolean isValidTableName(String tableName) {
354+
if (tableName == null || tableName.trim().isEmpty()) {
355+
return false;
356+
}
357+
358+
// Allow only alphanumeric characters, underscores, and dots (for schema.table format)
359+
// This prevents SQL injection via table name parameter
360+
return tableName.matches("^[a-zA-Z0-9_.]+$");
354361
}
355362
}

0 commit comments

Comments
 (0)