Validate and canonicalize Upload-Checksum value to prevent path traversal vulnerability

pom.xml

@@ -1,4 +1,6 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <groupId>me.desair.tus</groupId>
@@ -31,7 +33,7 @@
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-lang3</artifactId>
-            <version>[3.7, 3.99)</version>
+            <version>[3.18, 3.99)</version>
         </dependency>
         <dependency>
             <groupId>commons-io</groupId>

src/main/java/me/desair/tus/server/HttpMethod.java

@@ -1,9 +1,10 @@
 package me.desair.tus.server;
 
 import jakarta.servlet.http.HttpServletRequest;
+import java.util.Objects;
 import java.util.Set;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.commons.lang3.Validate;
+import org.apache.commons.lang3.Strings;
 
 /**
  * Class that represents a HTTP method. The X-HTTP-Method-Override request header MUST be a string
@@ -26,7 +27,7 @@ public enum HttpMethod {
   /** Get the {@link HttpMethod} instance that matches the provided name. */
   public static HttpMethod forName(String name) {
     for (HttpMethod method : HttpMethod.values()) {
-      if (StringUtils.equalsIgnoreCase(method.name(), name)) {
+      if (Strings.CI.equals(method.name(), name)) {
         return method;
       }
     }
@@ -40,7 +41,7 @@ public static HttpMethod forName(String name) {
    */
   public static HttpMethod getMethodIfSupported(
       HttpServletRequest request, Set<HttpMethod> supportedHttpMethods) {
-    Validate.notNull(request, "The HttpServletRequest cannot be null");
+    Objects.requireNonNull(request, "The HttpServletRequest cannot be null");
 
     String requestMethod = request.getHeader(HttpHeader.METHOD_OVERRIDE);
     if (StringUtils.isBlank(requestMethod) || forName(requestMethod) == null) {

src/main/java/me/desair/tus/server/TusFileUploadService.java

@@ -8,6 +8,7 @@
 import java.util.EnumSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.Objects;
 import java.util.Set;
 import me.desair.tus.server.checksum.ChecksumExtension;
 import me.desair.tus.server.concatenation.ConcatenationExtension;
@@ -29,7 +30,7 @@
 import me.desair.tus.server.util.TusServletRequest;
 import me.desair.tus.server.util.TusServletResponse;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 import org.apache.commons.lang3.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -104,7 +105,7 @@ public TusFileUploadService withMaxUploadSize(Long maxUploadSize) {
    * @return The current service
    */
   public TusFileUploadService withUploadIdFactory(UploadIdFactory uploadIdFactory) {
-    Validate.notNull(uploadIdFactory, "The UploadIdFactory cannot be null");
+    Objects.requireNonNull(uploadIdFactory, "The UploadIdFactory cannot be null");
     String previousUploadUri = this.idFactory.getUploadUri();
     this.idFactory = uploadIdFactory;
     this.idFactory.setUploadUri(previousUploadUri);
@@ -121,7 +122,7 @@ public TusFileUploadService withUploadIdFactory(UploadIdFactory uploadIdFactory)
    * @return The current service
    */
   public TusFileUploadService withUploadStorageService(UploadStorageService uploadStorageService) {
-    Validate.notNull(uploadStorageService, "The UploadStorageService cannot be null");
+    Objects.requireNonNull(uploadStorageService, "The UploadStorageService cannot be null");
     // Copy over any previous configuration
     uploadStorageService.setMaxUploadSize(this.uploadStorageService.getMaxUploadSize());
     uploadStorageService.setUploadExpirationPeriod(
@@ -142,7 +143,7 @@ public TusFileUploadService withUploadStorageService(UploadStorageService upload
    * @return The current service
    */
   public TusFileUploadService withUploadLockingService(UploadLockingService uploadLockingService) {
-    Validate.notNull(uploadLockingService, "The UploadStorageService cannot be null");
+    Objects.requireNonNull(uploadLockingService, "The UploadStorageService cannot be null");
     uploadLockingService.setIdFactory(this.idFactory);
     // Update the upload storage service
     this.uploadLockingService = uploadLockingService;
@@ -236,7 +237,7 @@ public TusFileUploadService withUploadDeduplication(boolean isEnabled) {
    * @return The current service
    */
   public TusFileUploadService addTusExtension(TusExtension feature) {
-    Validate.notNull(feature, "A custom feature cannot be null");
+    Objects.requireNonNull(feature, "A custom feature cannot be null");
     enabledFeatures.put(feature.getName(), feature);
     updateSupportedHttpMethods();
     return this;
@@ -251,9 +252,9 @@ public TusFileUploadService addTusExtension(TusExtension feature) {
    * @return The current service
    */
   public TusFileUploadService disableTusExtension(String extensionName) {
-    Validate.notNull(extensionName, "The extension name cannot be null");
+    Objects.requireNonNull(extensionName, "The extension name cannot be null");
     Validate.isTrue(
-        !StringUtils.equals("core", extensionName), "The core protocol cannot be disabled");
+        !Strings.CS.equals("core", extensionName), "The core protocol cannot be disabled");
 
     enabledFeatures.remove(extensionName);
     updateSupportedHttpMethods();
@@ -306,8 +307,8 @@ public void process(HttpServletRequest servletRequest, HttpServletResponse servl
   public void process(
       HttpServletRequest servletRequest, HttpServletResponse servletResponse, String ownerKey)
       throws IOException {
-    Validate.notNull(servletRequest, "The HTTP Servlet request cannot be null");
-    Validate.notNull(servletResponse, "The HTTP Servlet response cannot be null");
+    Objects.requireNonNull(servletRequest, "The HTTP Servlet request cannot be null");
+    Objects.requireNonNull(servletResponse, "The HTTP Servlet response cannot be null");
 
     HttpMethod method = HttpMethod.getMethodIfSupported(servletRequest, supportedHttpMethods);
 

src/main/java/me/desair/tus/server/checksum/ChecksumPatchRequestHandler.java

@@ -13,6 +13,7 @@
 import me.desair.tus.server.util.TusServletResponse;
 import me.desair.tus.server.util.Utils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 public class ChecksumPatchRequestHandler extends AbstractRequestHandler {
 
@@ -47,7 +48,7 @@ public void process(
         ChecksumAlgorithm checksumAlgorithm = checksumInfo.getAlgorithm();
         String calculatedValue = servletRequest.getCalculatedChecksum(checksumAlgorithm);
 
-        if (!StringUtils.equals(expectedValue, calculatedValue)) {
+        if (!Strings.CS.equals(expectedValue, calculatedValue)) {
           // throw an exception if the checksum is invalid. This will also trigger the removal
           // of any
           // bytes that were already saved

src/main/java/me/desair/tus/server/concatenation/ConcatenationPostRequestHandler.java

@@ -11,7 +11,7 @@
 import me.desair.tus.server.util.TusServletRequest;
 import me.desair.tus.server.util.TusServletResponse;
 import me.desair.tus.server.util.Utils;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /**
  * The Server MUST acknowledge a successful upload creation with the 201 Created status. The Server
@@ -41,10 +41,10 @@ public void process(
     if (uploadInfo != null) {
 
       String uploadConcatValue = servletRequest.getHeader(HttpHeader.UPLOAD_CONCAT);
-      if (StringUtils.equalsIgnoreCase(uploadConcatValue, "partial")) {
+      if (Strings.CI.equals(uploadConcatValue, "partial")) {
         uploadInfo.setUploadType(UploadType.PARTIAL);
 
-      } else if (StringUtils.startsWithIgnoreCase(uploadConcatValue, "final")) {
+      } else if (Strings.CI.startsWith(uploadConcatValue, "final")) {
         // reset the length, just to be sure
         uploadInfo.setLength(null);
         uploadInfo.setUploadType(UploadType.CONCATENATED);

src/main/java/me/desair/tus/server/concatenation/validation/NoUploadLengthOnFinalValidator.java

@@ -9,6 +9,7 @@
 import me.desair.tus.server.exception.UploadLengthNotAllowedOnConcatenationException;
 import me.desair.tus.server.upload.UploadStorageService;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /** The Client MUST NOT include the Upload-Length header in the upload creation. */
 public class NoUploadLengthOnFinalValidator implements RequestValidator {
@@ -23,7 +24,7 @@ public void validate(
 
     String uploadConcatValue = request.getHeader(HttpHeader.UPLOAD_CONCAT);
 
-    if (StringUtils.startsWithIgnoreCase(uploadConcatValue, "final")
+    if (Strings.CI.startsWith(uploadConcatValue, "final")
         && StringUtils.isNotBlank(request.getHeader(HttpHeader.UPLOAD_LENGTH))) {
 
       throw new UploadLengthNotAllowedOnConcatenationException(

src/main/java/me/desair/tus/server/concatenation/validation/PartialUploadsExistValidator.java

@@ -10,7 +10,7 @@
 import me.desair.tus.server.upload.UploadInfo;
 import me.desair.tus.server.upload.UploadStorageService;
 import me.desair.tus.server.util.Utils;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /** Validate that the IDs specified in the Upload-Concat header map to an existing upload */
 public class PartialUploadsExistValidator implements RequestValidator {
@@ -25,7 +25,7 @@ public void validate(
 
     String uploadConcatValue = request.getHeader(HttpHeader.UPLOAD_CONCAT);
 
-    if (StringUtils.startsWithIgnoreCase(uploadConcatValue, "final")) {
+    if (Strings.CI.startsWith(uploadConcatValue, "final")) {
 
       for (String uploadUri : Utils.parseConcatenationIDsFromHeader(uploadConcatValue)) {
 

src/main/java/me/desair/tus/server/core/validation/TusResumableValidator.java

@@ -9,7 +9,7 @@
 import me.desair.tus.server.exception.TusException;
 import me.desair.tus.server.upload.UploadStorageService;
 import me.desair.tus.server.util.Utils;
-import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /**
  * Class that will validate if the tus version in the request corresponds to our implementation
@@ -32,7 +32,7 @@ public void validate(
       throws TusException {
 
     String requestVersion = Utils.getHeader(request, HttpHeader.TUS_RESUMABLE);
-    if (!StringUtils.equals(requestVersion, TusFileUploadService.TUS_API_VERSION)) {
+    if (!Strings.CS.equals(requestVersion, TusFileUploadService.TUS_API_VERSION)) {
       throw new InvalidTusResumableException(
           "This server does not support tus protocol version " + requestVersion);
     }

src/main/java/me/desair/tus/server/core/validation/UploadOffsetValidator.java

@@ -12,6 +12,7 @@
 import me.desair.tus.server.upload.UploadStorageService;
 import me.desair.tus.server.util.Utils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /**
  * The Upload-Offset header’s value MUST be equal to the current offset of the resource. If the
@@ -34,7 +35,7 @@ public void validate(
 
     if (uploadInfo != null) {
       String expectedOffset = Objects.toString(uploadInfo.getOffset());
-      if (!StringUtils.equals(expectedOffset, uploadOffset)) {
+      if (!Strings.CS.equals(expectedOffset, uploadOffset)) {
         throw new UploadOffsetMismatchException(
             "The Upload-Offset was "
                 + StringUtils.trimToNull(uploadOffset)

src/main/java/me/desair/tus/server/creation/CreationPostRequestHandler.java

@@ -12,6 +12,7 @@
 import me.desair.tus.server.util.TusServletResponse;
 import me.desair.tus.server.util.Utils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -49,7 +50,7 @@ public void process(
     // It's important to return relative UPLOAD URLs in the Location header in order to support
     // HTTPS proxies
     // that sit in front of the web app
-    String url = uploadUri + (StringUtils.endsWith(uploadUri, "/") ? "" : "/") + info.getId();
+    String url = uploadUri + (Strings.CS.endsWith(uploadUri, "/") ? "" : "/") + info.getId();
     servletResponse.setHeader(HttpHeader.LOCATION, url);
     servletResponse.setStatus(HttpServletResponse.SC_CREATED);
 

src/main/java/me/desair/tus/server/creation/validation/UploadDeferLengthValidator.java

@@ -9,6 +9,7 @@
 import me.desair.tus.server.upload.UploadStorageService;
 import me.desair.tus.server.util.Utils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 
 /**
  * The request MUST include one of the following headers: a) Upload-Length to indicate the size of
@@ -37,7 +38,7 @@ public void validate(
     }
 
     String uploadConcatValue = request.getHeader(HttpHeader.UPLOAD_CONCAT);
-    if (StringUtils.startsWithIgnoreCase(uploadConcatValue, "final")) {
+    if (Strings.CI.startsWith(uploadConcatValue, "final")) {
       concatenatedUpload = true;
     }
 

src/main/java/me/desair/tus/server/upload/UploadIdFactory.java

@@ -4,6 +4,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Strings;
 import org.apache.commons.lang3.Validate;
 
 /**
@@ -24,8 +25,8 @@ public abstract class UploadIdFactory {
    */
   public void setUploadUri(String uploadUri) {
     Validate.notBlank(uploadUri, "The upload URI pattern cannot be blank");
-    Validate.isTrue(StringUtils.startsWith(uploadUri, "/"), "The upload URI should start with /");
-    Validate.isTrue(!StringUtils.endsWith(uploadUri, "$"), "The upload URI should not end with $");
+    Validate.isTrue(Strings.CS.startsWith(uploadUri, "/"), "The upload URI should start with /");
+    Validate.isTrue(!Strings.CS.endsWith(uploadUri, "$"), "The upload URI should not end with $");
     this.uploadUri = uploadUri;
     this.uploadUriPattern = null;
   }
@@ -86,7 +87,7 @@ protected Pattern getUploadUriPattern() {
       // We will extract the upload ID's by removing the upload URI from the start of the
       // request URI
       uploadUriPattern =
-          Pattern.compile("^.*" + uploadUri + (StringUtils.endsWith(uploadUri, "/") ? "" : "/?"));
+          Pattern.compile("^.*" + uploadUri + (Strings.CS.endsWith(uploadUri, "/") ? "" : "/?"));
     }
     return uploadUriPattern;
   }

src/main/java/me/desair/tus/server/upload/disk/DiskLockingService.java

@@ -9,6 +9,7 @@
 import java.nio.file.Path;
 import java.nio.file.attribute.FileTime;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import me.desair.tus.server.exception.TusException;
 import me.desair.tus.server.exception.UploadAlreadyLockedException;
@@ -17,7 +18,6 @@
 import me.desair.tus.server.upload.UploadLock;
 import me.desair.tus.server.upload.UploadLockingService;
 import me.desair.tus.server.util.InterruptibleInputStream;
-import org.apache.commons.lang3.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -50,7 +50,7 @@ public DiskLockingService(String storagePath) {
   /** Constructor to use custom UploadIdFactory. */
   public DiskLockingService(UploadIdFactory idFactory, String storagePath) {
     this(storagePath);
-    Validate.notNull(idFactory, "The IdFactory cannot be null");
+    Objects.requireNonNull(idFactory, "The IdFactory cannot be null");
     this.idFactory = idFactory;
   }
 
@@ -123,7 +123,7 @@ public boolean isLocked(UploadId id) {
 
   @Override
   public void setIdFactory(UploadIdFactory idFactory) {
-    Validate.notNull(idFactory, "The IdFactory cannot be null");
+    Objects.requireNonNull(idFactory, "The IdFactory cannot be null");
     this.idFactory = idFactory;
   }
 

src/main/java/me/desair/tus/server/upload/disk/DiskStorageService.java

@@ -32,7 +32,6 @@
 import me.desair.tus.server.upload.concatenation.VirtualConcatenationService;
 import me.desair.tus.server.util.Utils;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.lang3.Validate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -58,13 +57,13 @@ public DiskStorageService(String storagePath) {
 
   public DiskStorageService(UploadIdFactory idFactory, String storagePath) {
     this(storagePath);
-    Validate.notNull(idFactory, "The IdFactory cannot be null");
+    Objects.requireNonNull(idFactory, "The IdFactory cannot be null");
     this.idFactory = idFactory;
   }
 
   @Override
   public void setIdFactory(UploadIdFactory idFactory) {
-    Validate.notNull(idFactory, "The IdFactory cannot be null");
+    Objects.requireNonNull(idFactory, "The IdFactory cannot be null");
     this.idFactory = idFactory;
   }
 
@@ -324,7 +323,7 @@ public void setUploadExpirationPeriod(Long uploadExpirationPeriod) {
 
   @Override
   public void setUploadConcatenationService(UploadConcatenationService concatenationService) {
-    Validate.notNull(concatenationService);
+    Objects.requireNonNull(concatenationService);
     this.uploadConcatenationService = concatenationService;
   }
 

src/main/java/me/desair/tus/server/upload/disk/FileBasedLock.java

@@ -9,6 +9,7 @@
 import java.nio.channels.OverlappingFileLockException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Objects;
 import me.desair.tus.server.exception.UploadAlreadyLockedException;
 import me.desair.tus.server.upload.UploadLock;
 import me.desair.tus.server.util.Utils;
@@ -35,7 +36,7 @@ public class FileBasedLock implements UploadLock {
   public FileBasedLock(String uploadUri, Path lockPath)
       throws UploadAlreadyLockedException, IOException {
     Validate.notBlank(uploadUri, "The upload URI cannot be blank");
-    Validate.notNull(lockPath, "The path to the lock cannot be null");
+    Objects.requireNonNull(lockPath, "The path to the lock cannot be null");
     this.uploadUri = uploadUri;
     this.lockPath = lockPath;