Adding parameter checks

3 files changed, 45 insertions, 15 deletions

diff --git a/archiva-modules/archiva-web/archiva-web-common/src/main/java/org/apache/archiva/web/api/DefaultFileUploadService.java b/archiva-modules/archiva-web/archiva-web-common/src/main/java/org/apache/archiva/web/api/DefaultFileUploadService.java
index d5f0ec54e..4fd8f6808 100644
--- a/archiva-modules/archiva-web/archiva-web-common/src/main/java/org/apache/archiva/web/api/DefaultFileUploadService.java
+++ b/archiva-modules/archiva-web/archiva-web-common/src/main/java/org/apache/archiva/web/api/DefaultFileUploadService.java
@@ -69,9 +69,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Paths;
-import java.nio.file.StandardCopyOption;
+import java.nio.file.*;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -107,6 +105,8 @@ public class DefaultFileUploadService
 
     private ChecksumAlgorithm[] algorithms = new ChecksumAlgorithm[]{ ChecksumAlgorithm.SHA1, ChecksumAlgorithm.MD5 };
 
+    private final String FS = FileSystems.getDefault().getSeparator();
+
     @Inject
     @Named(value = "archivaTaskScheduler#repository")
     private ArchivaTaskScheduler scheduler;
@@ -136,6 +136,14 @@ public class DefaultFileUploadService
 
             //Content-Disposition: form-data; name="files[]"; filename="org.apache.karaf.features.command-2.2.2.jar"
             String fileName = file.getContentDisposition().getParameter( "filename" );
+            Path fileNamePath = Paths.get(fileName);
+            if (!fileName.equals(fileNamePath.getFileName().toString())) {
+                ArchivaRestServiceException e = new ArchivaRestServiceException("Bad filename in upload content: " + fileName + " - File traversal chars (..|/) are not allowed"
+                        , null);
+                e.setHttpErrorCode(422);
+                e.setErrorKey("error.upload.malformed.filename");
+                throw e;
+            }
 
             File tmpFile = File.createTempFile( "upload-artifact", ".tmp" );
             tmpFile.deleteOnExit();
@@ -224,6 +232,29 @@ public class DefaultFileUploadService
         return fileMetadatas == null ? Collections.<FileMetadata>emptyList() : fileMetadatas;
     }
 
+    private boolean hasValidChars(String checkString) {
+        if (checkString.contains(FS)) {
+            return false;
+        }
+        if (checkString.contains("../")) {
+            return false;
+        }
+        if (checkString.contains("/..")) {
+            return false;
+        }
+        return true;
+    }
+
+    private void checkParamChars(String param, String value) throws ArchivaRestServiceException {
+        if (!hasValidChars(value)) {
+            ArchivaRestServiceException e = new ArchivaRestServiceException("Bad characters in " + param, null);
+            e.setHttpErrorCode(422);
+            e.setErrorKey("error.upload.malformed.param." + param);
+            e.setFieldName(param);
+            throw e;
+        }
+    }
+
     @Override
     public Boolean save( String repositoryId, String groupId, String artifactId, String version, String packaging,
                          boolean generatePom )
@@ -235,6 +266,11 @@ public class DefaultFileUploadService
         version = StringUtils.trim( version );
         packaging = StringUtils.trim( packaging );
 
+        checkParamChars("repositoryId", repositoryId);
+        checkParamChars("groupId", groupId);
+        checkParamChars("artifactId", artifactId);
+        checkParamChars("packaging", packaging);
+
         List<FileMetadata> fileMetadatas = getSessionFilesList();
         if ( fileMetadatas == null || fileMetadatas.isEmpty() )
         {
diff --git a/archiva-modules/archiva-web/archiva-web-common/src/test/java/org/apache/archiva/upload/UploadArtifactsTest.java b/archiva-modules/archiva-web/archiva-web-common/src/test/java/org/apache/archiva/upload/UploadArtifactsTest.java
index d0f63b1af..d55f866e7 100644
--- a/archiva-modules/archiva-web/archiva-web-common/src/test/java/org/apache/archiva/upload/UploadArtifactsTest.java
+++ b/archiva-modules/archiva-web/archiva-web-common/src/test/java/org/apache/archiva/upload/UploadArtifactsTest.java
@@ -68,6 +68,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.ClientErrorException;
 import java.io.File;
 import java.io.IOException;
 import java.net.URLEncoder;
@@ -238,9 +239,9 @@ public class UploadArtifactsTest
                 service.post( body );
                 fail( "FileNames with path contents should not be allowed." );
             }
-            catch ( ArchivaRestServiceException e )
+            catch ( ClientErrorException e )
             {
-                // OK
+                assertEquals(422, e.getResponse().getStatus());
             }
         }
         finally
@@ -349,15 +350,15 @@ public class UploadArtifactsTest
             log.debug( "Metadata {}", meta.toString( ) );
             assertTrue( service.save( "internal", "org.archiva", "archiva-model", "1.2", "jar", true ) );
 
-            fileAttachment = new AttachmentBuilder( ).object( Files.newInputStream( file ) ).contentDisposition( new ContentDisposition( "form-data; filename=\"/../TestFile.FileExt\"; name=\"files[]\"" ) ).build( );
+            fileAttachment = new AttachmentBuilder( ).object( Files.newInputStream( file ) ).contentDisposition( new ContentDisposition( "form-data; filename=\"TestFile.FileExt\"; name=\"files[]\"" ) ).build( );
             body = new MultipartBody( fileAttachment );
             meta = service.post( body );
             log.debug( "Metadata {}", meta.toString( ) );
             try {
                 service.save("internal", "org", URLEncoder.encode("../../../test", "UTF-8"), URLEncoder.encode("testSave", "UTF-8"), "4", true);
                 fail("Error expected, if the content contains bad characters.");
-            } catch (ArchivaRestServiceException e) {
-                // OK
+            } catch (ClientErrorException e) {
+                assertEquals(422, e.getResponse().getStatus());
             }
             assertFalse( Files.exists( Paths.get( "target/test-testSave.4" ) ) );
         }
diff --git a/archiva-modules/archiva-web/archiva-web-common/src/test/resources/spring-context-test-upload.xml b/archiva-modules/archiva-web/archiva-web-common/src/test/resources/spring-context-test-upload.xml
index 0e2e156e7..e15e19eff 100644
--- a/archiva-modules/archiva-web/archiva-web-common/src/test/resources/spring-context-test-upload.xml
+++ b/archiva-modules/archiva-web/archiva-web-common/src/test/resources/spring-context-test-upload.xml
@@ -72,13 +72,6 @@
     </constructor-arg>
   </bean>
 
-  <bean name="taskQueueExecutor#repository-scanning"
-        class="org.apache.archiva.redback.components.taskqueue.execution.ThreadedTaskQueueExecutor" lazy-init="false">
-    <property name="name" value="repository-scanning"/>
-    <property name="executor" ref="taskExecutor#repository-scanning"/>
-    <property name="queue" ref="taskQueue#repository-scanning"/>
-  </bean>
-
   <bean id="repository" class="org.apache.jackrabbit.core.RepositoryImpl" destroy-method="shutdown">
     <constructor-arg ref="config"/>
   </bean>