CVE-2021-41152: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: OpenOlat is a web-based e-learning platform for teaching, learning, assessment and communication, an LMS, a learning management system. In affected versions by manipulating the HTTP request an attacker can modify the path of a requested file download in the folder component to point to anywhere on the target system. The attack could be used to read any file accessible in the web root folder or outside, depending on the configuration of the system and the properly configured permission of the application server user. The attack requires an OpenOlat user account or the enabled guest user feature together with the usage of the folder component in a course. The attack does not allow writing of arbitrary files, it allows only reading of files and also only ready of files that the attacker knows the exact path which is very unlikely at least for OpenOlat data files. The problem is fixed in version 15.5.8 and 16.0.1 It is advised to upgrade to version 16.0.x. There are no known workarounds to fix this problem, an upgrade is necessary.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.7
Published: 2021-10-18T21:15Z
Repository: https://github.com/OpenOLAT/OpenOLAT
Language: Java
Files changed: src/main/java/org/olat/core/commons/modules/bc/FileSelection.java, src/main/java/org/olat/core/commons/modules/bc/commands/CmdDelete.java, src/main/java/org/olat/core/commons/modules/bc/commands/CmdDownloadZip.java, src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java, src/main/java/org/olat/core/commons/modules/bc/commands/CmdUnzip.java, src/main/java/org/olat/core/commons/modules/bc/commands/CmdZip.java, src/main/java/org/olat/core/util/mail/ui/SendDocumentsByEMailController.java
Commit date: 2021-09-06 19:02:57 +0200
Commit message: OO-5696: validate file selections against current container
Diff: diff --git a/src/main/java/org/olat/core/commons/modules/bc/FileSelection.java b/src/main/java/org/olat/core/commons/modules/bc/FileSelection.java
index 6ce216023f..aa079de5c5 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/FileSelection.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/FileSelection.java
@@ -25,28 +25,33 @@
 */ 
 
 package org.olat.core.commons.modules.bc;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.olat.core.gui.UserRequest;
 import org.olat.core.util.FileUtils;
 import org.olat.core.util.StringHelper;
+import org.olat.core.util.vfs.VFSContainer;
+import org.olat.core.util.vfs.VFSItem;
 
 public class FileSelection {
 
 	/** HTML form identifier */
 	public static final String FORM_ID = "paths";
 	
-	private List<String> files = new ArrayList<>();
-	private String currentContainerRelPath;
+	private final List<String> files = new ArrayList<>();
+	private final String currentContainerRelPath;
+	private final VFSContainer currentContainer;
 	
-	public FileSelection(UserRequest ureq, String currentContainerRelPath) {
+	public FileSelection(UserRequest ureq, VFSContainer currentContainer, String currentContainerRelPath) {
 		if (currentContainerRelPath.equals("/")) currentContainerRelPath = "";
 		this.currentContainerRelPath = currentContainerRelPath;
+		this.currentContainer = currentContainer;
 		parse(ureq);
 	}
 
 	public List<String> getFiles() {
 		return files;
@@ -74,12 +79,24 @@ public class FileSelection {
 	 * @param base
 	 * @param ureq
 	 */
 	private void parse(UserRequest ureq) {
 		String[] sFiles = ureq.getHttpReq().getParameterValues(FORM_ID);
-		if (sFiles == null || sFiles.length == 0) return;
-		files = Arrays.asList(sFiles);
+		if (sFiles == null || sFiles.length == 0) {
+			return;
+		}
+		List<VFSItem> items = currentContainer.getItems();
+		if(items != null && !items.isEmpty()) {
+			Set<String> itemNames =  items.stream()
+					.map(VFSItem::getName)
+					.collect(Collectors.toSet());
+			for(String sFile:sFiles) {
+				if(itemNames.contains(sFile)) {
+					files.add(sFile);
+				}
+			}
+		}
 	}
 
 	/**
 	 * Render pathset as HTML.
 	 * 
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDelete.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDelete.java
index cf42cd3a7c..48d838a10c 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDelete.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDelete.java
@@ -72,11 +72,11 @@ public class CmdDelete extends BasicController implements FolderCommand {
 
 	@Override
 	public Controller execute(FolderComponent fc, UserRequest ureq, WindowControl wContr, Translator trans) {
 		this.translator = trans;
 		this.folderComponent = fc;
-		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainerPath());
+		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainer(), fc.getCurrentContainerPath());
 
 		VFSContainer currentContainer = folderComponent.getCurrentContainer();
 		List<String> lockedFiles = hasLockedFiles(currentContainer, fileSelection);
 		if (lockedFiles.isEmpty()) {
 			String msg = trans.translate("del.confirm") + "<p>" + fileSelection.renderAsHtml() + "</p>";		
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDownloadZip.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDownloadZip.java
index 53112a9385..520e57bf1d 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDownloadZip.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdDownloadZip.java
@@ -68,19 +68,19 @@ public class CmdDownloadZip implements FolderCommand {
 		status = FolderCommandHelper.sanityCheck(wControl, folderComponent);
 		if(status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
 	
-		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
+		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
 		status = FolderCommandHelper.sanityCheck3(wControl, folderComponent, selection);
 		if(status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
 		
 		if(selection.getFiles().isEmpty()) {
 			status = FolderCommandStatus.STATUS_FAILED;
-			wControl.setWarning(trans.translate("warning.file.selection.empty"));
+			wControl.setWarning(trans.translate("warning.file.selection.empty22"));
 			return null;
 		}
 		
 		MediaResource mr = new ZipMediaResource(currentContainer, selection);
 		ureq.getDispatchResult().setResultingMediaResource(mr);
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java
index d9388b4376..995ffb1612 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java
@@ -87,11 +87,11 @@ public class CmdMoveCopy extends DefaultController implements FolderCommand {
 	
 	@Override
 	public Controller execute(FolderComponent fc, UserRequest ureq, WindowControl windowControl, Translator trans) {
 		this.folderComponent = fc;
 		this.translator = trans;
-		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainerPath());
+		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainer(), fc.getCurrentContainerPath());
 				
 		VelocityContainer main = new VelocityContainer("mc", VELOCITY_ROOT + "/movecopy.html", translator, this);
 		main.contextPut("fileselection", fileSelection);
 		
 		//check if command is executed on a file list containing invalid filenames or paths
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUnzip.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUnzip.java
index 88d14a80ea..4db910aee3 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUnzip.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUnzip.java
@@ -75,11 +75,11 @@ public class CmdUnzip extends BasicController implements FolderCommand {
 	}
 	
 	@Override
 	public Controller execute(FolderComponent folderComponent, UserRequest ureq, WindowControl wContr, Translator trans) {
 		this.translator = trans;
-		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
+		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
 		VFSContainer currentContainer = folderComponent.getCurrentContainer();
 		if (currentContainer.canWrite() != VFSConstants.YES)
 			throw new AssertException("Cannot unzip to folder. Writing denied.");
 			
 	  //check if command is executed on a file containing invalid filenames or paths - checks if the resulting folder has a valid name
diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdZip.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdZip.java
index ef8d4e8e29..8618907956 100644
--- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdZip.java
+++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdZip.java
@@ -87,11 +87,11 @@ public class CmdZip extends FormBasicController implements FolderCommand {
 		status = FolderCommandHelper.sanityCheck(wControl, folderComponent);
 		if(status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
 	
-		selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
+		selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
 		status = FolderCommandHelper.sanityCheck3(wControl, folderComponent, selection);
 		if(status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
 		
diff --git a/src/main/java/org/olat/core/util/mail/ui/SendDocumentsByEMailController.java b/src/main/java/org/olat/core/util/mail/ui/SendDocumentsByEMailController.java
index 0365cdd997..9dcbdcb8f2 100644
--- a/src/main/java/org/olat/core/util/mail/ui/SendDocumentsByEMailController.java
+++ b/src/main/java/org/olat/core/util/mail/ui/SendDocumentsByEMailController.java
@@ -187,11 +187,11 @@ public class SendDocumentsByEMailController extends FormBasicController implemen
 		}
 		status = FolderCommandHelper.sanityCheck(wControl, folderComponent);
 		if (status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
-		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
+		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
 		status = FolderCommandHelper.sanityCheck3(wControl, folderComponent, selection);
 		if (status == FolderCommandStatus.STATUS_FAILED) {
 			return null;
 		}
 

Vulnerable code: import java.util.Arrays;
	private List<String> files = new ArrayList<>();
	private String currentContainerRelPath;
	public FileSelection(UserRequest ureq, String currentContainerRelPath) {
		if (sFiles == null || sFiles.length == 0) return;
		files = Arrays.asList(sFiles);
		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
			wControl.setWarning(trans.translate("warning.file.selection.empty"));
		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
		selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainerPath());
Fixed code: import java.util.Set;
import java.util.stream.Collectors;
import org.olat.core.util.vfs.VFSContainer;
import org.olat.core.util.vfs.VFSItem;
	private final List<String> files = new ArrayList<>();
	private final String currentContainerRelPath;
	private final VFSContainer currentContainer;
	public FileSelection(UserRequest ureq, VFSContainer currentContainer, String currentContainerRelPath) {
		this.currentContainer = currentContainer;
		if (sFiles == null || sFiles.length == 0) {
			return;
		}
		List<VFSItem> items = currentContainer.getItems();
		if(items != null && !items.isEmpty()) {
			Set<String> itemNames =  items.stream()
					.map(VFSItem::getName)
					.collect(Collectors.toSet());
			for(String sFile:sFiles) {
				if(itemNames.contains(sFile)) {
					files.add(sFile);
				}
			}
		}
		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainer(), fc.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
			wControl.setWarning(trans.translate("warning.file.selection.empty22"));
		this.fileSelection = new FileSelection(ureq, fc.getCurrentContainer(), fc.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
		selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());
		FileSelection selection = new FileSelection(ureq, folderComponent.getCurrentContainer(), folderComponent.getCurrentContainerPath());