CVE-2021-21245: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: OneDev is an all-in-one devops platform. In OneDev before version 4.0.3, AttachmentUploadServlet also saves user controlled data (`request.getInputStream()`) to a user specified location (`request.getHeader("File-Name")`). This issue may lead to arbitrary file upload which can be used to upload a WebShell to OneDev server. This issue is addressed in 4.0.3 by only allowing uploaded file to be in attachments folder. The webshell issue is not possible as OneDev never executes files in attachments folder.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 9.8
Published: 2021-01-15T21:15Z
Repository: https://github.com/theonedev/onedev
Language: Java
Files changed: server-core/src/main/java/io/onedev/server/util/FilenameUtils.java, server-core/src/main/java/io/onedev/server/web/component/markdown/InsertUrlPanel.java, server-core/src/main/java/io/onedev/server/web/component/markdown/MarkdownEditor.java, server-core/src/main/java/io/onedev/server/web/page/project/blob/ProjectBlobPage.java
Commit date: 2020-11-20 10:08:14 +0800
Commit message: Fix the issue that uploaded file can be stored anywhere OneDev has write
permissions over
Diff: diff --git a/server-core/src/main/java/io/onedev/server/util/FilenameUtils.java b/server-core/src/main/java/io/onedev/server/util/FilenameUtils.java
new file mode 100644
index 0000000000..91d10b7842
--- /dev/null
+++ b/server-core/src/main/java/io/onedev/server/util/FilenameUtils.java
@@ -0,0 +1,9 @@
+package io.onedev.server.util;
+
+public class FilenameUtils extends org.apache.commons.io.FilenameUtils {
+
+	public static String sanitizeFilename(String fileName) {
+		return fileName.replace("..", "_").replace('/', '_').replace('\\', '_');
+	}
+
+}
\ No newline at end of file
diff --git a/server-core/src/main/java/io/onedev/server/web/component/markdown/InsertUrlPanel.java b/server-core/src/main/java/io/onedev/server/web/component/markdown/InsertUrlPanel.java
index 9a972075e8..d55c180952 100644
--- a/server-core/src/main/java/io/onedev/server/web/component/markdown/InsertUrlPanel.java
+++ b/server-core/src/main/java/io/onedev/server/web/component/markdown/InsertUrlPanel.java
@@ -51,10 +51,11 @@ import io.onedev.commons.utils.PathUtils;
 import io.onedev.commons.utils.StringUtils;
 import io.onedev.server.git.BlobIdent;
 import io.onedev.server.git.BlobIdentFilter;
 import io.onedev.server.git.exception.GitException;
 import io.onedev.server.model.Project;
+import io.onedev.server.util.FilenameUtils;
 import io.onedev.server.util.UrlUtils;
 import io.onedev.server.web.ajaxlistener.ConfirmClickListener;
 import io.onedev.server.web.behavior.ReferenceInputBehavior;
 import io.onedev.server.web.component.blob.folderpicker.BlobFolderPicker;
 import io.onedev.server.web.component.blob.picker.BlobPicker;
@@ -400,11 +401,12 @@ abstract class InsertUrlPanel extends Panel {
 					
 					AjaxRequestTarget target = RequestCycle.get().find(AjaxRequestTarget.class);
 					String attachmentName;
 					FileUpload upload = uploads.iterator().next();
 					try (InputStream is = upload.getInputStream()) {
-						attachmentName = attachmentSupport.saveAttachment(upload.getClientFileName(), is);
+						attachmentName = attachmentSupport.saveAttachment(
+								FilenameUtils.sanitizeFilename(upload.getClientFileName()), is);
 					} catch (IOException e) {
 						throw new RuntimeException(e);
 					}
 					markdownEditor.insertUrl(target, isImage, 
 							attachmentSupport.getAttachmentUrl(attachmentName), UrlUtils.describe(attachmentName), null);
diff --git a/server-core/src/main/java/io/onedev/server/web/component/markdown/MarkdownEditor.java b/server-core/src/main/java/io/onedev/server/web/component/markdown/MarkdownEditor.java
index 371e157b25..024ee69e29 100644
--- a/server-core/src/main/java/io/onedev/server/web/component/markdown/MarkdownEditor.java
+++ b/server-core/src/main/java/io/onedev/server/web/component/markdown/MarkdownEditor.java
@@ -58,10 +58,11 @@ import io.onedev.server.entitymanager.ProjectManager;
 import io.onedev.server.model.Build;
 import io.onedev.server.model.Issue;
 import io.onedev.server.model.Project;
 import io.onedev.server.model.PullRequest;
 import io.onedev.server.model.User;
+import io.onedev.server.util.FilenameUtils;
 import io.onedev.server.util.markdown.MarkdownManager;
 import io.onedev.server.util.validation.ProjectNameValidator;
 import io.onedev.server.web.avatar.AvatarManager;
 import io.onedev.server.web.behavior.AbstractPostAjaxBehavior;
 import io.onedev.server.web.component.floating.FloatingPanel;
@@ -475,11 +476,12 @@ public class MarkdownEditor extends FormComponentPanel<String> {
 			protected void respond(AjaxRequestTarget target) {
 				Preconditions.checkNotNull(getAttachmentSupport(), "Unexpected attachment upload request");
 				HttpServletRequest request = (HttpServletRequest) RequestCycle.get().getRequest().getContainerRequest();
 				HttpServletResponse response = (HttpServletResponse) RequestCycle.get().getResponse().getContainerResponse();
 				try {
-					String fileName = URLDecoder.decode(request.getHeader("File-Name"), StandardCharsets.UTF_8.name());
+					String fileName = FilenameUtils.sanitizeFilename(
+							URLDecoder.decode(request.getHeader("File-Name"), StandardCharsets.UTF_8.name()));
 					String attachmentName = getAttachmentSupport().saveAttachment(fileName, request.getInputStream());
 					response.getWriter().print(URLEncoder.encode(attachmentName, StandardCharsets.UTF_8.name()));
 					response.setStatus(HttpServletResponse.SC_OK);
 				} catch (Exception e) {
 					logger.error("Error uploading attachment.", e);
diff --git a/server-core/src/main/java/io/onedev/server/web/page/project/blob/ProjectBlobPage.java b/server-core/src/main/java/io/onedev/server/web/page/project/blob/ProjectBlobPage.java
index e0bb4a4db3..bb085149f6 100644
--- a/server-core/src/main/java/io/onedev/server/web/page/project/blob/ProjectBlobPage.java
+++ b/server-core/src/main/java/io/onedev/server/web/page/project/blob/ProjectBlobPage.java
@@ -83,10 +83,11 @@ import io.onedev.server.search.code.IndexManager;
 import io.onedev.server.search.code.SearchManager;
 import io.onedev.server.search.code.hit.QueryHit;
 import io.onedev.server.search.code.query.BlobQuery;
 import io.onedev.server.search.code.query.TextQuery;
 import io.onedev.server.security.SecurityUtils;
+import io.onedev.server.util.FilenameUtils;
 import io.onedev.server.util.script.identity.JobIdentity;
 import io.onedev.server.util.script.identity.ScriptIdentity;
 import io.onedev.server.util.script.identity.ScriptIdentityAware;
 import io.onedev.server.web.PrioritizedComponentRenderer;
 import io.onedev.server.web.behavior.AbstractPostAjaxBehavior;
@@ -1411,11 +1412,11 @@ public class ProjectBlobPage extends ProjectPage implements BlobRenderContext, S
 		
 		User user = Preconditions.checkNotNull(SecurityUtils.getUser());
 		BlobIdent blobIdent = getBlobIdent();
 		
 		for (FileUpload upload: uploads) {
-			String blobPath = upload.getClientFileName();
+			String blobPath = FilenameUtils.sanitizeFilename(upload.getClientFileName());
 			if (parentPath != null)
 				blobPath = parentPath + "/" + blobPath;
 			
 			if (getProject().isReviewRequiredForModification(user, blobIdent.revision, blobPath)) 
 				throw new BlobEditException("Review required for this change. Please submit pull request instead");

Vulnerable code: 						attachmentName = attachmentSupport.saveAttachment(upload.getClientFileName(), is);
					String fileName = URLDecoder.decode(request.getHeader("File-Name"), StandardCharsets.UTF_8.name());
			String blobPath = upload.getClientFileName();
Fixed code: package io.onedev.server.util;

public class FilenameUtils extends org.apache.commons.io.FilenameUtils {

	public static String sanitizeFilename(String fileName) {
		return fileName.replace("..", "_").replace('/', '_').replace('\\', '_');
	}

}
import io.onedev.server.util.FilenameUtils;
						attachmentName = attachmentSupport.saveAttachment(
								FilenameUtils.sanitizeFilename(upload.getClientFileName()), is);
import io.onedev.server.util.FilenameUtils;
					String fileName = FilenameUtils.sanitizeFilename(
							URLDecoder.decode(request.getHeader("File-Name"), StandardCharsets.UTF_8.name()));
import io.onedev.server.util.FilenameUtils;
			String blobPath = FilenameUtils.sanitizeFilename(upload.getClientFileName());