TSK-1103 Changed vulnerable logging messages

TSK-1103 Make method sanitizeLoggingMessage generic

TSK-1103 Renamed method

lib/taskana-core/src/main/java/pro/taskana/classification/internal/ClassificationServiceImpl.java

@@ -30,6 +30,7 @@ import pro.taskana.common.api.exceptions.NotAuthorizedException;
 import pro.taskana.common.internal.InternalTaskanaEngine;
 import pro.taskana.common.internal.jobs.ClassificationChangedJob;
 import pro.taskana.common.internal.util.IdGenerator;
+import pro.taskana.common.internal.util.LogSanitizer;
 import pro.taskana.task.api.models.TaskSummary;
 import pro.taskana.task.internal.TaskMapper;
 
@@ -221,7 +222,7 @@ public class ClassificationServiceImpl implements ClassificationService {
           InvalidArgumentException {
     LOGGER.debug("entry to updateClassification(Classification = {})", classification);
     taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN);
-    ClassificationImpl classificationImpl = null;
+    ClassificationImpl classificationImpl;
     try {
       taskanaEngine.openConnection();
       if (classification.getKey().equals(classification.getParentKey())) {
@@ -357,7 +358,7 @@ public class ClassificationServiceImpl implements ClassificationService {
         LOGGER.warn(
             "Method createClassification: Classification does already exist "
                 + "in master domain. Classification {}.",
-            masterClassification);
+            LogSanitizer.stripLineBreakingChars(masterClassification));
       } finally {
         if (!doesExist) {
           classificationMapper.insert(masterClassification);

lib/taskana-core/src/main/java/pro/taskana/common/internal/jobs/TaskCleanupJob.java

@@ -19,6 +19,7 @@ import pro.taskana.common.api.TimeInterval;
 import pro.taskana.common.api.exceptions.InvalidArgumentException;
 import pro.taskana.common.api.exceptions.TaskanaException;
 import pro.taskana.common.internal.transaction.TaskanaTransactionProvider;
+import pro.taskana.common.internal.util.LogSanitizer;
 import pro.taskana.task.api.models.TaskSummary;
 
 /** Job to cleanup completed tasks after a period of time. */
@@ -178,15 +179,15 @@ public class TaskCleanupJob extends AbstractTaskanaJob {
     }
 
     List<String> tasksIdsToBeDeleted =
-        tasksToBeDeleted.stream().map(task -> task.getId()).collect(Collectors.toList());
+        tasksToBeDeleted.stream().map(TaskSummary::getId).collect(Collectors.toList());
     BulkOperationResults<String, TaskanaException> results =
         taskanaEngineImpl.getTaskService().deleteTasks(tasksIdsToBeDeleted);
     LOGGER.debug("{} tasks deleted.", tasksIdsToBeDeleted.size() - results.getFailedIds().size());
     for (String failedId : results.getFailedIds()) {
       LOGGER.warn(
           "Task with id {} could not be deleted. Reason: {}",
-          failedId,
-          results.getErrorForId(failedId));
+          LogSanitizer.stripLineBreakingChars(failedId),
+          LogSanitizer.stripLineBreakingChars(results.getErrorForId(failedId)));
     }
     LOGGER.debug(
         "exit from deleteTasks(), returning {}",

lib/taskana-core/src/main/java/pro/taskana/common/internal/util/LogSanitizer.java

@@ -0,0 +1,18 @@
+package pro.taskana.common.internal.util;
+
+public class LogSanitizer {
+
+  private LogSanitizer() {
+    throw new IllegalStateException("Utility class");
+  }
+
+  /**
+   * Removes characters which break the log file pattern. Protects against injection attacks.
+   *
+   * @param loggingMessage String which should be sanitized
+   * @return sanitized logging message
+   */
+  public static String stripLineBreakingChars(Object loggingMessage) {
+    return loggingMessage.toString().replaceAll("[\n|\r|\t]", "_");
+  }
+}

lib/taskana-core/src/main/java/pro/taskana/task/internal/TaskRoutingManager.java

@@ -10,6 +10,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import pro.taskana.common.api.TaskanaEngine;
+import pro.taskana.common.internal.util.LogSanitizer;
 import pro.taskana.spi.routing.api.TaskRoutingProvider;
 import pro.taskana.task.api.models.Task;
 
@@ -75,9 +76,13 @@ public final class TaskRoutingManager {
               .filter(Objects::nonNull)
               .collect(Collectors.toSet());
       if (workbasketIds.isEmpty()) {
-        LOGGER.error("No TaskRouter determined a workbasket for task {}.", task);
+        LOGGER.error(
+            "No TaskRouter determined a workbasket for task {}.",
+            LogSanitizer.stripLineBreakingChars(task));
       } else if (workbasketIds.size() > 1) {
-        LOGGER.error("The TaskRouters determined more than one workbasket for task {}", task);
+        LOGGER.error(
+            "The TaskRouters determined more than one workbasket for task {}",
+            LogSanitizer.stripLineBreakingChars(task));
       } else {
         workbasketId = workbasketIds.stream().findFirst().orElse(null);
       }