You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@archiva.apache.org by ma...@apache.org on 2019/05/03 19:17:55 UTC
[archiva] 07/12: Adding parameter checks
This is an automated email from the ASF dual-hosted git repository.
martin_s pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/archiva.git
commit c347ee5541644e20d895daf4b25e32a5ecd6f9dd
Author: Martin Stockhammer <ma...@apache.org>
AuthorDate: Wed Feb 27 17:54:39 2019 +0100
Adding parameter checks
(cherry picked from commit 29e40eae69f945119b566d75dc3bdecd4a81edd5)
---
.../archiva/web/api/DefaultFileUploadService.java | 38 ++++++++++++++++++++++
.../apache/archiva/upload/UploadArtifactsTest.java | 12 +++----
.../test/resources/spring-context-test-upload.xml | 4 +--
3 files changed, 45 insertions(+), 9 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 0e55bdb..b4d60cc 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
@@ -108,6 +108,8 @@ public class DefaultFileUploadService
private List<ChecksumAlgorithm> algorithms = Arrays.asList( ChecksumAlgorithm.SHA1, ChecksumAlgorithm.MD5 );
+ private final String FS = FileSystems.getDefault().getSeparator();
+
@Inject
@Named(value = "archivaTaskScheduler#repository")
private ArchivaTaskScheduler<RepositoryTask> scheduler;
@@ -137,6 +139,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;
+ }
Path tmpFile = Files.createTempFile( "upload-artifact", ".tmp" );
tmpFile.toFile().deleteOnExit();
@@ -227,6 +237,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 )
@@ -238,6 +271,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 1b341f3..254d9bb 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
@@ -35,7 +35,7 @@ import org.apache.cxf.jaxrs.ext.multipart.MultipartBody;
import org.apache.cxf.message.Message;
import org.junit.Test;
import org.junit.runner.RunWith;
-
+import javax.ws.rs.ClientErrorException;
import java.io.IOException;
import java.net.URLEncoder;
import java.nio.file.Files;
@@ -130,9 +130,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
@@ -241,15 +241,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 dbdfa6b..5109a9b 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
@@ -81,8 +81,6 @@
<property name="queue" ref="taskQueue#repository-scanning"/>
</bean>
-
-
<alias name="repositorySessionFactory#jcr" alias="repositorySessionFactory"/>
<alias name="userConfiguration#archiva" alias="userConfiguration#default"/>
<alias name="authorizer#rbac" alias="authorizer#default"/>
@@ -120,4 +118,4 @@
End of JPA settings
*** -->
-</beans>
\ No newline at end of file
+</beans>