Skip to content

Commit e311e53

Browse files
committed
feat: enhance logging by sanitizing inputs in BackupService and SystemSettingsService
1 parent c9fbe22 commit e311e53

2 files changed

Lines changed: 51 additions & 20 deletions

File tree

src/main/java/com/crimeLink/analyzer/service/BackupService.java

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public BackupMetadata createBackup(String userEmail) {
4646
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd_HH-mm-ss"));
4747
String filename = "backup_" + timestamp + ".sql";
4848

49-
Path backupDir = Paths.get(backupDirectory);
50-
Path backupFile = backupDir.resolve(filename);
49+
Path backupFile = resolveSafeBackupPath(filename);
50+
Path backupDir = backupFile.getParent();
5151

5252
BackupMetadata metadata = new BackupMetadata();
5353
metadata.setFilename(filename);
@@ -86,12 +86,12 @@ public BackupMetadata createBackup(String userEmail) {
8686
backupRepo.save(metadata);
8787

8888
log.info("Backup created successfully: {} ({} bytes) by {}",
89-
filename, resultFile.length(), userEmail);
89+
sanitizeForLog(filename), resultFile.length(), sanitizeForLog(userEmail));
9090

9191
return metadata;
9292

9393
} catch (Exception e) {
94-
log.error("Backup failed: {}", e.getMessage(), e);
94+
log.error("Backup failed: {}", sanitizeForLog(e.getMessage()), e);
9595
metadata.setStatus("FAILED");
9696
metadata.setSizeBytes(0L);
9797
backupRepo.save(metadata);
@@ -103,13 +103,7 @@ public BackupMetadata createBackup(String userEmail) {
103103
* Restore database from a backup file by executing its SQL statements.
104104
*/
105105
public void restoreBackup(String filename, String userEmail) {
106-
// Sanitize filename to prevent path traversal
107-
if (!SAFE_FILENAME.matcher(filename).matches()) {
108-
throw new IllegalArgumentException(
109-
"Invalid filename. Only alphanumeric characters, underscores, hyphens, and .sql extension are allowed.");
110-
}
111-
112-
Path backupFile = Paths.get(backupDirectory).resolve(filename);
106+
Path backupFile = resolveSafeBackupPath(filename);
113107

114108
if (!Files.exists(backupFile)) {
115109
throw new IllegalArgumentException("Backup file not found: " + filename);
@@ -134,15 +128,16 @@ public void restoreBackup(String filename, String userEmail) {
134128
} catch (Exception stmtEx) {
135129
// Log and continue — some statements may fail on duplicates etc.
136130
log.warn("Skipping failed statement: {}... Error: {}",
137-
trimmed.substring(0, Math.min(80, trimmed.length())),
138-
stmtEx.getMessage());
131+
sanitizeForLog(trimmed.substring(0, Math.min(80, trimmed.length()))),
132+
sanitizeForLog(stmtEx.getMessage()));
139133
}
140134
}
141135

142-
log.info("Database restored from {} by {} ({} statements executed)", filename, userEmail, executed);
136+
log.info("Database restored from {} by {} ({} statements executed)",
137+
sanitizeForLog(filename), sanitizeForLog(userEmail), executed);
143138

144139
} catch (IOException e) {
145-
log.error("Restore failed: {}", e.getMessage(), e);
140+
log.error("Restore failed: {}", sanitizeForLog(e.getMessage()), e);
146141
throw new RuntimeException("Restore failed: " + e.getMessage(), e);
147142
}
148143
}
@@ -176,7 +171,7 @@ private List<String> getTableNames() {
176171
private void exportTableData(BufferedWriter writer, String tableName) throws IOException {
177172
// Validate table name to prevent SQL injection (should only contain safe chars)
178173
if (!tableName.matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) {
179-
log.warn("Skipping suspicious table name: {}", tableName);
174+
log.warn("Skipping suspicious table name: {}", sanitizeForLog(tableName));
180175
return;
181176
}
182177

@@ -214,12 +209,40 @@ private void exportTableData(BufferedWriter writer, String tableName) throws IOE
214209
+ String.join(", ", values) + ");\n");
215210
}
216211
} catch (Exception e) {
217-
log.error("Error exporting table {}: {}", tableName, e.getMessage());
212+
log.error("Error exporting table {}: {}", sanitizeForLog(tableName), sanitizeForLog(e.getMessage()));
218213
}
219214
});
220215
} catch (Exception e) {
221216
writer.write("-- ERROR exporting table " + tableName + ": " + e.getMessage() + "\n");
222-
log.error("Failed to export table {}: {}", tableName, e.getMessage());
217+
log.error("Failed to export table {}: {}", sanitizeForLog(tableName), sanitizeForLog(e.getMessage()));
223218
}
224219
}
220+
221+
/**
222+
* Confirms the requested filename is safe, resolves and normalizes the path,
223+
* and strictly checks that it falls inside the base backup directory.
224+
*/
225+
private Path resolveSafeBackupPath(String filename) {
226+
if (filename == null || !SAFE_FILENAME.matcher(filename).matches()) {
227+
throw new IllegalArgumentException(
228+
"Invalid filename. Only alphanumeric characters, underscores, hyphens, and .sql extension are allowed.");
229+
}
230+
231+
Path baseDir = Paths.get(backupDirectory).toAbsolutePath().normalize();
232+
Path resolvedPath = baseDir.resolve(filename).normalize();
233+
234+
if (!resolvedPath.startsWith(baseDir)) {
235+
throw new IllegalArgumentException("Path traversal attempt detected.");
236+
}
237+
238+
return resolvedPath;
239+
}
240+
241+
/**
242+
* Sanitizes strings for safe logging to prevent log injection.
243+
*/
244+
private String sanitizeForLog(String input) {
245+
if (input == null) return "null";
246+
return input.replaceAll("[\\r\\n\\t]", "_");
247+
}
225248
}

src/main/java/com/crimeLink/analyzer/service/SystemSettingsService.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public Map<String, String> updateSettings(Map<String, String> incoming) {
5858

5959
// Ignore unknown keys
6060
if (!DEFAULTS.containsKey(key)) {
61-
log.warn("Ignoring unknown setting key: {}", key);
61+
log.warn("Ignoring unknown setting key: {}", sanitizeForLog(key));
6262
continue;
6363
}
6464

@@ -89,7 +89,7 @@ public Map<String, String> updateSettings(Map<String, String> incoming) {
8989
settingRepo.save(setting);
9090
}
9191

92-
log.info("System settings updated: {}", incoming.keySet());
92+
log.info("System settings updated: {}", sanitizeForLog(incoming.keySet().toString()));
9393
return getAllSettings();
9494
}
9595

@@ -101,4 +101,12 @@ public String getSetting(String key) {
101101
.map(SystemSetting::getSettingValue)
102102
.orElse(DEFAULTS.get(key));
103103
}
104+
105+
/**
106+
* Sanitize input for logging by removing CRLF and other control characters.
107+
*/
108+
private String sanitizeForLog(String input) {
109+
if (input == null) return "null";
110+
return input.replaceAll("[\\r\\n\\t]", "_");
111+
}
104112
}

0 commit comments

Comments
 (0)