Changed logger to lo4j, log invalid login with username (log4shell in…

…put). Added commons-collection to do a deserialization RCE on newer java version based on log4shell.
snyk-labs · Jan 6, 2022 · 186ff5c · 186ff5c
1 parent 4625d86
commit 186ff5c
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 8 deletions.
diff --git a/...dolist-core/src/main/java/io/github/todolist/core/repository/impl/TodoRepositoryImpl.java b/...dolist-core/src/main/java/io/github/todolist/core/repository/impl/TodoRepositoryImpl.java
@@ -26,6 +26,7 @@
 
 import io.github.todolist.core.domain.Todo;
 import io.github.todolist.core.repository.api.TodoRepository;
+import org.apache.commons.collections.list.UnmodifiableList;
 import org.springframework.stereotype.Repository;
 
 import javax.persistence.EntityManager;
@@ -57,7 +58,7 @@ public Todo getTodoById(final long id) {
     public List<Todo> getTodoListByUser(final long userId) {
         TypedQuery<Todo> query = entityManager.createNamedQuery("findTodosByUser", Todo.class);
         query.setParameter(1, userId);
-        return query.getResultList();
+        return UnmodifiableList.decorate(query.getResultList());
     }
 
     /**
@@ -67,7 +68,7 @@ public List<Todo> getTodoListByUserAndTitle(final long userId, final String titl
         TypedQuery<Todo> query = entityManager.createNamedQuery("findTodosByTitle", Todo.class);
         query.setParameter(1, userId);
         query.setParameter(2, "%" + title.toUpperCase() + "%");
-        return query.getResultList();
+        return UnmodifiableList.decorate(query.getResultList());
     }
 
     /**

diff --git a/todolist-goof/todolist-web-common/pom.xml b/todolist-goof/todolist-web-common/pom.xml
@@ -62,5 +62,12 @@
             <version>4.3.1.Final</version>
         </dependency>
 
+        <!--vulnerable commons collections (deserialization) -->
+        <dependency>
+            <groupId>commons-collections</groupId>
+            <artifactId>commons-collections</artifactId>
+            <version>3.1</version>
+        </dependency>
+
     </dependencies>
 </project>
diff --git a/...list-web-common/src/main/java/io/github/benas/todolist/web/common/util/TodoListUtils.java b/...list-web-common/src/main/java/io/github/benas/todolist/web/common/util/TodoListUtils.java
@@ -94,6 +94,7 @@ public static String getStatusLabel(boolean status) {
      * @param input   text to which apply the style for each matched pattern
      * @param pattern the pattern to highlight
      * @return the transformed text
+     *
      */
     public static String highlight(final String input, final String pattern) {
 

diff --git a/...list-web-struts/src/main/java/io/github/benas/todolist/web/action/user/AccountAction.java b/...list-web-struts/src/main/java/io/github/benas/todolist/web/action/user/AccountAction.java
@@ -26,8 +26,8 @@
 
 import com.opensymphony.xwork2.Action;
 import com.opensymphony.xwork2.ActionSupport;
-import com.opensymphony.xwork2.util.logging.Logger;
-import com.opensymphony.xwork2.util.logging.LoggerFactory;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import io.github.benas.todolist.web.action.BaseAction;
 import io.github.benas.todolist.web.common.form.ChangePasswordForm;
 import io.github.benas.todolist.web.common.form.RegistrationForm;
@@ -45,7 +45,7 @@
  */
 public class AccountAction extends BaseAction {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(AccountAction.class.getName());
+    private static final Logger LOGGER = LogManager.getLogger(AccountAction.class.getName());
 
     private ChangePasswordForm changePasswordForm;
 

diff --git a/...list-web-struts/src/main/java/io/github/benas/todolist/web/action/user/SessionAction.java b/...list-web-struts/src/main/java/io/github/benas/todolist/web/action/user/SessionAction.java
@@ -25,8 +25,8 @@
 package io.github.benas.todolist.web.action.user;
 
 import com.opensymphony.xwork2.Action;
-import com.opensymphony.xwork2.util.logging.Logger;
-import com.opensymphony.xwork2.util.logging.LoggerFactory;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import io.github.benas.todolist.web.action.BaseAction;
 import io.github.benas.todolist.web.common.form.LoginForm;
 import io.github.benas.todolist.web.common.util.TodoListUtils;
@@ -39,7 +39,7 @@
  */
 public class SessionAction extends BaseAction {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(SessionAction.class.getName());
+    private static final Logger LOGGER = LogManager.getLogger(SessionAction.class.getName());
 
     private LoginForm loginForm;
 
@@ -61,6 +61,7 @@ public String doLogin() {
             session.put(TodoListUtils.SESSION_USER, user);
             return Action.SUCCESS;
         } else {
+            LOGGER.error("Login failed for email: " + loginForm.getEmail());
             error = getText("login.error.global.invalid");
             return Action.INPUT;
         }