This is an automated email from the ASF dual-hosted git repository. martin_s pushed a commit to branch archiva-2.x in repository https://gitbox.apache.org/repos/asf/archiva.git
commit 29e40eae69f945119b566d75dc3bdecd4a81edd5 Author: Martin Stockhammer <[email protected]> AuthorDate: Wed Feb 27 17:54:39 2019 +0100 Adding parameter checks --- .../archiva/web/api/DefaultFileUploadService.java | 42 ++++++++++++++++++++-- .../apache/archiva/upload/UploadArtifactsTest.java | 11 +++--- .../test/resources/spring-context-test-upload.xml | 7 ---- 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 d5f0ec5..4fd8f68 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 d0f63b1..d55f866 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 0e2e156..e15e19e 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>
