fix(security): Fix SQL injection vulnerabilities in ContainerFactoryImpl

dotCMS/src/main/java/com/dotcms/util/SecurityUtils.java

@@ -303,4 +303,100 @@ public void validateFile (final String fileName) {
     }
   }
 
+  /**
+   * Regular expression pattern for validating content type variable names.
+   * This pattern allows alphanumeric characters and underscores, starting with a letter or underscore.
+   * Made public to be reusable across the codebase.
+   */
+  public static final String VALID_VARIABLE_NAME_REGEX = "[_A-Za-z][_0-9A-Za-z]*";
+
+  /**
+   * Validates if an identifier is safe to use in SQL queries and other security-sensitive contexts.
+   * This method checks if the identifier is one of the following valid formats:
+   * <ul>
+   *   <li>A valid UUID format</li>
+   *   <li>A known system identifier (e.g., SYSTEM_HOST, SYSTEM_FOLDER, SYSTEM_CONTAINER, SYSTEM_TEMPLATE, SYSTEM_THEME)</li>
+   *   <li>A valid content type variable name (alphanumeric with underscores, starting with letter/underscore)</li>
+   * </ul>
+   *
+   * @param identifier the identifier to validate
+   * @throws SecurityException if the identifier is null, empty, or does not match any valid format
+   */
+  public static void validateIdentifier(final String identifier) throws SecurityException {
+    if (!UtilMethods.isSet(identifier)) {
+      throw new SecurityException("Identifier cannot be null or empty");
+    }
+
+    if (!isValidIdentifier(identifier)) {
+      throw new SecurityException(
+          String.format("Invalid identifier format: '%s'. Must be a valid UUID, system identifier (SYSTEM_HOST, SYSTEM_FOLDER, SYSTEM_CONTAINER, SYSTEM_TEMPLATE, SYSTEM_THEME), or content type variable name.",
+                       identifier));
+    }
+  }
+
+  /**
+   * Checks if an identifier is valid without throwing an exception.
+   * An identifier is considered valid if it is:
+   * <ul>
+   *   <li>A valid UUID format</li>
+   *   <li>A known system identifier (SYSTEM_HOST, SYSTEM_FOLDER, SYSTEM_CONTAINER, SYSTEM_TEMPLATE, SYSTEM_THEME)</li>
+   *   <li>A valid content type variable name</li>
+   * </ul>
+   *
+   * @param identifier the identifier to check
+   * @return true if the identifier is valid, false otherwise
+   */
+  public static boolean isValidIdentifier(final String identifier) {
+    if (!UtilMethods.isSet(identifier)) {
+      return false;
+    }
+
+    // Check if it's a UUID
+    if (com.dotmarketing.util.UUIDUtil.isUUID(identifier)) {
+      return true;
+    }
+
+    // Check if it's a known system identifier
+    if (isSystemIdentifier(identifier)) {
+      return true;
+    }
+
+    // Check if it matches valid variable name pattern
+    if (identifier.matches(VALID_VARIABLE_NAME_REGEX)) {
+      return true;
+    }
+
+    return false;
+  }
+
+  /**
+   * Checks if the identifier is a known system identifier.
+   * System identifiers are special constants used internally by dotCMS.
+   *
+   * @param identifier the identifier to check
+   * @return true if it's a known system identifier, false otherwise
+   */
+  public static boolean isSystemIdentifier(final String identifier) {
+    return "SYSTEM_HOST".equals(identifier) ||
+           "SYSTEM_FOLDER".equals(identifier) ||
+           "SYSTEM_CONTAINER".equals(identifier) ||
+           "SYSTEM_TEMPLATE".equals(identifier) ||
+           "SYSTEM_THEME".equals(identifier);
+  }
+
+  /**
+   * Validates if a string is a valid content type variable name.
+   * Variable names must start with a letter or underscore, followed by any combination
+   * of letters, numbers, or underscores.
+   *
+   * @param variableName the variable name to validate
+   * @return true if the variable name is valid, false otherwise
+   */
+  public static boolean isValidVariableName(final String variableName) {
+    if (!UtilMethods.isSet(variableName)) {
+      return false;
+    }
+    return variableName.matches(VALID_VARIABLE_NAME_REGEX);
+  }
+
 }

dotCMS/src/main/java/com/dotcms/util/pagination/ContainerPaginator.java

@@ -140,13 +140,13 @@ private PaginatedArrayList<Container> sortByTypeAndHost(
         }
 
         if (direction == OrderDirection.ASC) {
-            systemContainers.stream().sorted(Comparator.comparing(this::hostname));
-            dbContainers.stream().sorted(Comparator.comparing(this::hostname));
-            fileContainers.stream().sorted(Comparator.comparing(this::hostname));
+            systemContainers.sort(Comparator.comparing(this::hostname));
+            dbContainers.sort(Comparator.comparing(this::hostname));
+            fileContainers.sort(Comparator.comparing(this::hostname));
         } else {
-            systemContainers.stream().sorted(Comparator.comparing(this::hostname).reversed());
-            dbContainers.stream().sorted(Comparator.comparing(this::hostname).reversed());
-            fileContainers.stream().sorted(Comparator.comparing(this::hostname).reversed());
+            systemContainers.sort(Comparator.comparing(this::hostname).reversed());
+            dbContainers.sort(Comparator.comparing(this::hostname).reversed());
+            fileContainers.sort(Comparator.comparing(this::hostname).reversed());
         }
 
         final PaginatedArrayList<Container> sortedByHostContainers = new PaginatedArrayList<>();

dotCMS/src/main/java/com/dotmarketing/portlets/containers/business/ContainerFactoryImpl.java

@@ -515,7 +515,7 @@ public List<Container> findContainers(final User user, final boolean includeArch
 	public List<Container> findContainers(final User user, final ContainerAPI.SearchParams searchParams) throws DotSecurityException, DotDataException {
 		final ContentTypeAPI contentTypeAPI        = APILocator.getContentTypeAPI(user);
 		final StringBuffer conditionBuffer         = new StringBuffer();
-		final List<Object> paramValues 			   = this.getConditionParametersAndBuildConditionQuery(searchParams.filteringCriteria(), conditionBuffer);
+		final List<Object> paramValues 			   = new ArrayList<>();
 		final PaginatedArrayList<Container> assets = new PaginatedArrayList<>();
 		final List<Permissionable> toReturn        = new ArrayList<>();
 		int     internalLimit                      = 500;
@@ -526,7 +526,8 @@ public List<Container> findContainers(final User user, final ContainerAPI.Search
 				.append(Type.CONTAINERS.getTableName()).append(" asset, inode, identifier, ")
 				.append(Type.CONTAINERS.getVersionTableName()).append(" vinfo");
 
-		this.buildFindContainersQuery(searchParams, contentTypeAPI, query);
+		this.buildFindContainersQuery(searchParams, contentTypeAPI, query, paramValues);
+		paramValues.addAll(this.getConditionParametersAndBuildConditionQuery(searchParams.filteringCriteria(), conditionBuffer));
 
 		orderBy = UtilMethods.isEmpty(orderBy) ? "mod_date desc" : orderBy;
 
@@ -547,6 +548,9 @@ public List<Container> findContainers(final User user, final ContainerAPI.Search
 				}
 			}
 
+			Logger.debug(this, String.format("Finding containers with query: %s, params: %s, offset: %d, limit: %d",
+				query.toString(), paramValues, searchParams.offset(), searchParams.limit()));
+
 			// Adding Containers as Files located in the /application/containers/ folder
 			toReturn.addAll(this.findFolderAssetContainers(user, searchParams));
 
@@ -564,7 +568,13 @@ public List<Container> findContainers(final User user, final ContainerAPI.Search
 				internalOffset += internalLimit;
 			}
 
+			Logger.debug(this, String.format("Found %d total containers before pagination, applying offset=%d, limit=%d",
+				toReturn.size(), searchParams.offset(), searchParams.limit()));
+
 			getPaginatedAssets(searchParams.offset(), searchParams.limit(), assets, toReturn);
+
+			Logger.debug(this, String.format("Returning %d paginated containers", assets.size()));
+
 			if (searchParams.includeSystemContainer()) {
 				// System Container is being included, so increase the total result count by 1
 				assets.setTotalResults(assets.getTotalResults() + 1L);
@@ -665,8 +675,12 @@ private Collection<? extends Permissionable> findFolderAssetContainers(final Use
 				}).collect(Collectors.toList());
 			}
 
-			if (UtilMethods.isSet(searchParams.orderBy())) {
-				switch (searchParams.orderBy().toLowerCase()) {
+			// Sanitize orderBy parameter to prevent SQL injection
+			String orderBy = SQLUtil.sanitizeSortBy(searchParams.orderBy());
+			orderBy = UtilMethods.isEmpty(orderBy) ? "mod_date desc" : orderBy;
+
+			if (UtilMethods.isSet(orderBy)) {
+				switch (orderBy.toLowerCase()) {
 					case "title asc":
 						containers.sort(Comparator.comparing(Container::getTitle));
 					break;
@@ -676,12 +690,21 @@ private Collection<? extends Permissionable> findFolderAssetContainers(final Use
                         break;
 
 					case "moddate asc":
+					case "mod_date asc":
 						containers.sort(Comparator.comparing(Container::getModDate));
 						break;
 
+					case "moddate":
+					case "mod_date":
                     case "moddate desc":
+                    case "mod_date desc":
                         containers.sort(Comparator.comparing(Container::getModDate).reversed());
                         break;
+
+					default:
+						// For malicious or unknown orderBy values, use default mod_date desc sorting
+						containers.sort(Comparator.comparing(Container::getModDate).reversed());
+						break;
 				}
 			}
 
@@ -848,36 +871,48 @@ private List<Container> getFolderContainers(final Host host, final User user,
 
 	/**
 	 * Builds the main part of the SQL query that will be used to find Containers in the dotCMS repository.
+	 * Uses parameterized queries to prevent SQL injection attacks.
 	 *
 	 * @param searchParams   User-specified search criteria.
 	 * @param contentTypeAPI An instance of the {@link ContentTypeAPI}.
 	 * @param query          The SQL query being built.
+	 * @param paramValues    List to collect the query parameters for parameterized execution.
 	 *
-	 * @throws DotSecurityException The user cannot perform this action.
+	 * @throws DotSecurityException The user cannot perform this action or invalid input detected.
 	 * @throws DotDataException     An error occurred when interacting with the data source.
 	 */
 	private void buildFindContainersQuery(final ContainerAPI.SearchParams searchParams, final ContentTypeAPI contentTypeAPI,
-										  final StringBuilder query) throws DotSecurityException, DotDataException {
+										  final StringBuilder query, final List<Object> paramValues) throws DotSecurityException, DotDataException {
 
 		if(UtilMethods.isSet(searchParams.contentTypeIdOrVar())) {
 
 			//Search for the given ContentType inode
 			final ContentType foundContentType = contentTypeAPI.find(searchParams.contentTypeIdOrVar());
 
 			if (null != foundContentType && InodeUtils.isSet(foundContentType.inode())) {
+				// Use parameterized query to prevent SQL injection
 				query.append(
 								" where asset.inode = inode.inode and asset.identifier = identifier.id")
 						.append(
 								" and exists (select * from container_structures cs where cs.container_id = asset.identifier")
-						.append(" and cs.structure_id = '")
-						.append(foundContentType.inode())
-						.append("' ) ");
+						.append(" and cs.structure_id = ? ) ");
+				// Validate that inode is a valid UUID
+				final String inode = foundContentType.inode();
+				if (!UtilMethods.isSet(inode) || !UUIDUtil.isUUID(inode)) {
+					throw new DotSecurityException("Invalid inode format: " + inode);
+				}
+				paramValues.add(inode);
 			}else {
+				// Use parameterized query to prevent SQL injection
 				query.append(
 								" ,tree where asset.inode = inode.inode and asset.identifier = identifier.id")
-						.append(" and tree.parent = '")
-						.append(searchParams.contentTypeIdOrVar())
-						.append("' and tree.child=asset.inode");
+						.append(" and tree.parent = ? and tree.child=asset.inode");
+				// Validate the contentTypeIdOrVar to prevent injection (UUID, variable name, or identifier)
+				final String contentTypeIdOrVar = searchParams.contentTypeIdOrVar();
+				if (!UtilMethods.isSet(contentTypeIdOrVar) || !com.dotcms.util.SecurityUtils.isValidIdentifier(contentTypeIdOrVar)) {
+					throw new DotSecurityException("Invalid content type identifier: " + contentTypeIdOrVar);
+				}
+				paramValues.add(contentTypeIdOrVar);
 			}
 		} else {
 			query.append(" where asset.inode = inode.inode and asset.identifier = identifier.id");
@@ -891,19 +926,42 @@ private void buildFindContainersQuery(final ContainerAPI.SearchParams searchPara
 		}
 
 		if(UtilMethods.isSet(searchParams.siteId())) {
-			query.append(" and identifier.host_inode = '");
-			query.append(searchParams.siteId()).append('\'');
+			// Use parameterized query to prevent SQL injection
+			query.append(" and identifier.host_inode = ?");
+			// Validate siteId format (UUID or special identifiers like SYSTEM_HOST)
+			final String siteId = searchParams.siteId();
+			try {
+				com.dotcms.util.SecurityUtils.validateIdentifier(siteId);
+			} catch (SecurityException e) {
+				throw new DotSecurityException("Invalid site ID format: " + siteId, e);
+			}
+			paramValues.add(siteId);
 		}
 
 		if(UtilMethods.isSet(searchParams.containerInode())) {
-			query.append(" and asset.inode = '");
-			query.append(searchParams.containerInode()).append('\'');
+			// Use parameterized query to prevent SQL injection
+			query.append(" and asset.inode = ?");
+			// Validate containerInode format (UUID or system identifier)
+			final String containerInode = searchParams.containerInode();
+			try {
+				com.dotcms.util.SecurityUtils.validateIdentifier(containerInode);
+			} catch (SecurityException e) {
+				throw new DotSecurityException("Invalid container inode format: " + containerInode, e);
+			}
+			paramValues.add(containerInode);
 		}
 
 		if(UtilMethods.isSet(searchParams.containerIdentifier())) {
-			query.append(" and asset.identifier = '");
-			query.append(searchParams.containerIdentifier());
-			query.append('\'');
+			// Use parameterized query to prevent SQL injection
+			query.append(" and asset.identifier = ?");
+			// Validate containerIdentifier format (UUID or special identifier)
+			final String containerIdentifier = searchParams.containerIdentifier();
+			try {
+				com.dotcms.util.SecurityUtils.validateIdentifier(containerIdentifier);
+			} catch (SecurityException e) {
+				throw new DotSecurityException("Invalid container identifier format: " + containerIdentifier, e);
+			}
+			paramValues.add(containerIdentifier);
 		}
 	}
 
@@ -918,14 +976,13 @@ private void buildFindContainersQuery(final ContainerAPI.SearchParams searchPara
 	 *
 	 * @return The values for each specific search parameter.
 	 */
-	private List<Object> getConditionParametersAndBuildConditionQuery(final Map<String, Object> params, final StringBuffer conditionQueryBuffer) {
+	private List<Object> getConditionParametersAndBuildConditionQuery(final Map<String, Object> params, final StringBuffer conditionQueryBuffer) throws DotSecurityException {
 
-		List<Object> paramValues = null;
+		final List<Object> paramValues = new ArrayList<>();
 
 		if (params != null && !params.isEmpty()) {
 
 			conditionQueryBuffer.append(" and (");
-			paramValues = new ArrayList<>();
 			int counter = 0;
 
 			for (final Map.Entry<String, Object> entry : params.entrySet()) {
@@ -960,7 +1017,7 @@ private List<Object> getConditionParametersAndBuildConditionQuery(final Map<Stri
 	private void buildConditionParameterAndBuildConditionQuery (final Map.Entry<String, Object> entry,
 																final List<Object> paramValues,
 																final StringBuffer conditionQueryBuffer,
-																final Optional<String> prefix) {
+																final Optional<String> prefix) throws DotSecurityException {
 
 		if(entry.getValue() instanceof String){
 			if (entry.getKey().equalsIgnoreCase("inode") || entry.getKey()
@@ -971,9 +1028,13 @@ private void buildConditionParameterAndBuildConditionQuery (final Map.Entry<Stri
 				}
 				conditionQueryBuffer.append(" asset.");
 				conditionQueryBuffer.append(entry.getKey());
-				conditionQueryBuffer.append(" = '");
-				conditionQueryBuffer.append(entry.getValue());
-				conditionQueryBuffer.append('\'');
+				conditionQueryBuffer.append(" = ?");
+				// Validate identifier/inode format to prevent SQL injection
+				final String value = (String) entry.getValue();
+				if (!UtilMethods.isSet(value) || !com.dotcms.util.SecurityUtils.isValidIdentifier(value)) {
+					throw new DotSecurityException("Invalid " + entry.getKey() + " format: " + value);
+				}
+				paramValues.add(value);
 			} else {
 
 				if (prefix.isPresent()) {

dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowFactoryImpl.java

@@ -90,7 +90,7 @@ public class WorkflowFactoryImpl implements WorkFlowFactory {
     private static final String WA_USE_ROLE_HIERARCHY_ASSIGN_COLUMN = "use_role_hierarchy_assign";
     private static final String WA_METADATA_COLUMN = "metadata";
 
-    private static final String VALID_VARIABLE_NAME_REGEX = "[_A-Za-z][_0-9A-Za-z]*";
+    public static final String VALID_VARIABLE_NAME_REGEX = "[_A-Za-z][_0-9A-Za-z]*";
 
     /**
      * Creates an instance of the {@link WorkFlowFactory}.