#23900 fixing ajax vulnerabilities reported by sonar (#23911)

* #23900 fixing ajax vulnerabilities reported by sonar * #23900 clean up import
dotCMS · Jan 27, 2023 · fb67760 · fb67760
1 parent aa04d26
commit fb67760
Show file tree

Hide file tree

Showing 11 changed files with 451 additions and 37 deletions.
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/osgi/AJAX/OSGIAJAX.java b/dotCMS/src/main/java/com/dotmarketing/portlets/osgi/AJAX/OSGIAJAX.java
@@ -18,6 +18,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
+import java.util.Set;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -266,4 +267,19 @@ private void remove () {
 
     }
 
+    @Override
+    protected Set<String> getAllowedCommands() {
+        return Set.of(
+                "remove",
+                "restart",
+                "modifyExtraPackages",
+                "getExtraPackages",
+                "add",
+                "start",
+                "stop",
+                "deploy",
+                "undeploy",
+                "action"
+        );
+    }
 }
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/osgi/AJAX/OSGIBaseAJAX.java b/dotCMS/src/main/java/com/dotmarketing/portlets/osgi/AJAX/OSGIBaseAJAX.java
@@ -3,14 +3,19 @@
 import com.dotcms.api.web.HttpServletRequestThreadLocal;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.cms.factories.PublicCompanyFactory;
+import com.dotmarketing.exception.DotDataException;
 import com.dotmarketing.exception.DotRuntimeException;
 import com.dotmarketing.exception.DotSecurityException;
 import com.dotmarketing.servlets.ajax.AjaxAction;
 import com.dotmarketing.util.Logger;
+import com.dotmarketing.util.UtilMethods;
 import com.dotmarketing.util.WebKeys;
+import com.google.common.annotations.VisibleForTesting;
 import com.liferay.portal.language.LanguageUtil;
 import com.liferay.portal.model.User;
 import java.io.IOException;
+import java.lang.reflect.Method;
+import java.util.Set;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -31,9 +36,9 @@ public boolean validateUser() {
             throw new DotRuntimeException (e.getMessage());
         }
     }
-    
-    
-    
+
+
+	protected abstract Set<String> getAllowedCommands();
 
 	public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
 
@@ -46,20 +51,23 @@ public void service(HttpServletRequest request, HttpServletResponse response) th
 		Class partypes[] = new Class[] { HttpServletRequest.class, HttpServletResponse.class };
 		Object arglist[] = new Object[] { request, response };
 		try {
-		    if(getUser()==null || !APILocator.getLayoutAPI().doesUserHaveAccessToPortlet("dynamic-plugins", getUser())){
+		    if(getUser()==null || !doesUserHaveAccessToPortlet(getUser())){
 				response.sendError(401);
 				return;
 			}
-
-
 
-			meth = this.getClass().getMethod(cmd, partypes);
+			if(getAllowedCommands().contains(cmd)) {
+				meth = getMethod(cmd, partypes);
+			}	else  if (UtilMethods.isSet(cmd)){
+				Logger.error(this.getClass(), String.format("Attempt to run Invalid command %s :",cmd));
+				return;
+			}
 
 		} catch (Exception e) {
 
 			try {
 				cmd = "action";
-				meth = this.getClass().getMethod(cmd, partypes);
+				meth = getMethod(cmd, partypes);
 			} catch (Exception ex) {
 				Logger.error(this.getClass(), "Trying to run method:" + cmd);
 				Logger.error(this.getClass(), e.getMessage(), e.getCause());
@@ -77,6 +85,30 @@ public void service(HttpServletRequest request, HttpServletResponse response) th
 
 	}
 
+	/**
+	 * Extracted so it can be mocked
+	 * @param user
+	 * @return
+	 * @throws DotDataException
+	 */
+	@VisibleForTesting
+	boolean doesUserHaveAccessToPortlet(final User user) throws DotDataException {
+	  return APILocator.getLayoutAPI().doesUserHaveAccessToPortlet("dynamic-plugins", user);
+	}
+
+	/**
+	 * extracted as a separated method, so it can be verified when called
+	 * @param method
+	 * @param parameterTypes
+	 * @return
+	 * @throws NoSuchMethodException
+	 */
+	@VisibleForTesting
+	Method getMethod(String method, Class<?>... parameterTypes ) throws NoSuchMethodException {
+		return this.getClass().getMethod(method, parameterTypes);
+	}
+
+
 	public void writeError(HttpServletResponse response, String error) throws IOException {
 		String ret = null;
 

diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfActionAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfActionAjax.java
@@ -18,6 +18,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
 
 
 @Deprecated
@@ -137,4 +138,15 @@ public void save(final HttpServletRequest request,
             writeError(response, e.getMessage());
         }
     } // save.
+
+	/**
+	 * Security check demanded by Sonar
+	 * We register all the allowed methods down here
+	 *
+	 * @return allowed method names
+	 */
+	@Override
+	protected Set<String> getAllowedCommands() {
+		return Set.of( "action", "reorder", "delete", "add", "save", "deleteActionForStep" );
+	}
 }
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfActionClassAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfActionClassAjax.java
@@ -1,14 +1,5 @@
 package com.dotmarketing.portlets.workflows.ajax;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.web.UserWebAPI;
 import com.dotmarketing.business.web.WebAPILocator;
@@ -21,6 +12,15 @@
 import com.dotmarketing.util.Logger;
 import com.liferay.portal.model.User;
 
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 @Deprecated
 public class WfActionClassAjax extends WfBaseAction {
 
@@ -135,5 +135,16 @@ public void save(final HttpServletRequest request, final HttpServletResponse res
 			writeError(response, e.getMessage());
 		}
 	}
+
+	/**
+	 * Security check demanded by Sonar
+	 * We register all the allowed methods down here
+	 *
+	 * @return allowed method names
+	 */
+	@Override
+	protected Set<String> getAllowedCommands() {
+		return Set.of("save","add","delete","reorder","action");
+	}
 }
 
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfBaseAction.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfBaseAction.java
@@ -1,6 +1,8 @@
 package com.dotmarketing.portlets.workflows.ajax;
 
 import java.io.IOException;
+import java.lang.reflect.Method;
+import java.util.Set;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -9,11 +11,15 @@
 import com.dotmarketing.cms.factories.PublicCompanyFactory;
 import com.dotmarketing.servlets.ajax.AjaxAction;
 import com.dotmarketing.util.Logger;
+import com.dotmarketing.util.UtilMethods;
+import com.google.common.annotations.VisibleForTesting;
 import com.liferay.portal.language.LanguageUtil;
 
 @Deprecated
 abstract class WfBaseAction extends AjaxAction {
 
+	protected abstract Set<String> getAllowedCommands();
+
 	public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
 		String cmd = request.getParameter("cmd");
 		java.lang.reflect.Method meth = null;
@@ -25,13 +31,18 @@ public void service(HttpServletRequest request, HttpServletResponse response) th
 				return;
 			}
 
-			meth = this.getClass().getMethod(cmd, partypes);
+		  if(getAllowedCommands().contains(cmd)) {
+			  meth = getMethod(cmd, partypes);
+		  }	else  if (UtilMethods.isSet(cmd)){
+			  Logger.error(this.getClass(), String.format("Attempt to run Invalid command %s :",cmd));
+			  return;
+		  }
 
 		} catch (Exception e) {
 
 			try {
 				cmd = "action";
-				meth = this.getClass().getMethod(cmd, partypes);
+				meth = getMethod(cmd, partypes);
 			} catch (Exception ex) {
 				Logger.error(this.getClass(), "Trying to run method:" + cmd);
 				Logger.error(this.getClass(), e.getMessage(), e.getCause());
@@ -49,6 +60,18 @@ public void service(HttpServletRequest request, HttpServletResponse response) th
 
 	}
 
+	/**
+	 * extracted as a separated method, so it can be verified when called
+	 * @param method
+	 * @param parameterTypes
+	 * @return
+	 * @throws NoSuchMethodException
+	 */
+	@VisibleForTesting
+	Method getMethod(String method,  Class<?>... parameterTypes ) throws NoSuchMethodException {
+		return this.getClass().getMethod(method, parameterTypes);
+	}
+
 	public void writeError(HttpServletResponse response, String error) throws IOException {
 		String ret = null;
 

diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfRoleStoreAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfRoleStoreAjax.java
@@ -1,7 +1,5 @@
 package com.dotmarketing.portlets.workflows.ajax;
 
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.Role;
 import com.dotmarketing.business.RoleAPI;
@@ -10,20 +8,19 @@
 import com.dotmarketing.portlets.workflows.model.WorkflowAction;
 import com.dotmarketing.util.Logger;
 import com.dotmarketing.util.UtilMethods;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.liferay.portal.language.LanguageException;
 import com.liferay.portal.language.LanguageUtil;
 import com.liferay.portal.model.User;
 import com.liferay.util.StringPool;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import org.apache.commons.beanutils.BeanUtils;
+
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.beanutils.BeanUtils;
+import java.io.IOException;
+import java.util.*;
 
 @Deprecated
 public class WfRoleStoreAjax extends WfBaseAction {
@@ -247,4 +244,15 @@ private String rolesToJson ( List<Role> roles, boolean includeFake, boolean incl
         return mapper.writerWithDefaultPrettyPrinter().writeValueAsString( m );
     }
 
+    /**
+     * Security check demanded by Sonar
+     * We register all the allowed methods down here
+     *
+     * @return allowed method names
+     */
+    @Override
+    protected Set<String> getAllowedCommands() {
+        return Set.of( "rolesToJson", "assignable", "action" );
+    }
+
 }
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfSchemeAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfSchemeAjax.java
@@ -15,6 +15,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.util.Set;
 
 @Deprecated
 public class WfSchemeAjax extends WfBaseAction {
@@ -58,4 +59,15 @@ private String saveScheme(String schemeId, WorkflowSchemeForm schemeForm, User u
         return "SUCCESS";
     }
 
+    /**
+     * Security check demanded by Sonar
+     * We register all the allowed methods down here
+     *
+     * @return allowed method names
+     */
+    @Override
+    protected Set<String> getAllowedCommands() {
+        return Set.of( "save", "action" );
+    }
+
 }
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfStepAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/workflows/ajax/WfStepAjax.java
@@ -1,8 +1,6 @@
 package com.dotmarketing.portlets.workflows.ajax;
 
 import com.dotcms.exception.ExceptionUtil;
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.dotcms.workflow.form.WorkflowActionStepBean;
 import com.dotcms.workflow.form.WorkflowStepAddForm;
 import com.dotcms.workflow.form.WorkflowStepUpdateForm;
@@ -15,6 +13,8 @@
 import com.dotmarketing.portlets.workflows.model.WorkflowStep;
 import com.dotmarketing.util.Logger;
 import com.dotmarketing.util.StringUtils;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.liferay.portal.model.User;
 
 import javax.servlet.ServletException;
@@ -200,7 +200,16 @@ private String stepsToJson(List<WorkflowStep> steps) throws IOException{
         m.put("items", list);
 		return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(m);
 	}
-
-
+
+	/**
+	 * Security check demanded by Sonar
+	 * We register all the allowed methods down here
+	 *
+	 * @return allowed method names
+	 */
+	@Override
+	protected Set<String> getAllowedCommands() {
+		return Set.of( "listByScheme", "add", "addActionToStep", "delete", "reorder", "action" );
+	}
 
 }