From 8a13d818fe05aa13055fc1208da4764be109f872 Mon Sep 17 00:00:00 2001 From: Martin Stockhammer Date: Sat, 13 Apr 2019 11:59:29 +0200 Subject: [PATCH] [MRM-1987] Improving URL check for organisation info (cherry picked from commit 796716d44183bd315dd20184a66b39ae533eb747) This is the final commit from the 2.x branch of multiple commits to fix the vulnerabilities CVE-2019-0213 and CVE-2019-0214 --- .../admin/DefaultArchivaAdministration.java | 27 +++++----- .../admin/ArchivaAdministrationTest.java | 4 +- .../DefaultArchivaAdministrationService.java | 8 +-- .../ArchivaAdministrationServiceTest.java | 49 +++++++++++++++++++ 4 files changed, 67 insertions(+), 21 deletions(-) diff --git a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java index 0c8a682a6..3be9f58dd 100644 --- a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java +++ b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/main/java/org/apache/archiva/admin/repository/admin/DefaultArchivaAdministration.java @@ -21,17 +21,12 @@ package org.apache.archiva.admin.repository.admin; import org.apache.archiva.admin.model.AuditInformation; import org.apache.archiva.admin.model.RepositoryAdminException; import org.apache.archiva.admin.model.admin.ArchivaAdministration; -import org.apache.archiva.admin.model.beans.FileType; -import org.apache.archiva.admin.model.beans.LegacyArtifactPath; -import org.apache.archiva.admin.model.beans.NetworkConfiguration; -import org.apache.archiva.admin.model.beans.OrganisationInformation; -import org.apache.archiva.admin.model.beans.UiConfiguration; +import org.apache.archiva.admin.model.beans.*; import org.apache.archiva.admin.repository.AbstractRepositoryAdmin; import org.apache.archiva.configuration.Configuration; import org.apache.archiva.configuration.UserInterfaceOptions; import org.apache.archiva.configuration.WebappConfiguration; import org.apache.archiva.metadata.model.facets.AuditEvent; -import org.apache.commons.codec.net.URLCodec; import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.StringUtils; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; @@ -41,10 +36,8 @@ import org.springframework.util.ResourceUtils; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; -import java.io.UnsupportedEncodingException; -import java.net.MalformedURLException; -import java.net.URL; -import java.net.URLEncoder; +import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -327,14 +320,21 @@ public class DefaultArchivaAdministration return getModelMapper().map( organisationInformation, OrganisationInformation.class ); } - private void checkUrl(String url, String propertyName) throws RepositoryAdminException { + private String fixUrl(String url, String propertyName) throws RepositoryAdminException { if ( StringUtils.isNotEmpty( url ) ) { if ( !ResourceUtils.isUrl( url ) ) { throw new RepositoryAdminException( "Bad URL in " + propertyName + ": " + url ); } + try { + URI urlToCheck = new URI(url); + return urlToCheck.toString(); + } catch (URISyntaxException e) { + throw new RepositoryAdminException( "Bad URL in " + propertyName + ": " + url ); + } } + return url; } @@ -346,8 +346,9 @@ public class DefaultArchivaAdministration public void setOrganisationInformation( OrganisationInformation organisationInformation ) throws RepositoryAdminException { - checkUrl(organisationInformation.getUrl(), "url"); - checkUrl( organisationInformation.getLogoLocation(), "logoLocation" ); + + organisationInformation.setUrl(fixUrl(organisationInformation.getUrl(), "url")); + organisationInformation.setLogoLocation(fixUrl( organisationInformation.getLogoLocation(), "logoLocation" )); Configuration configuration = getArchivaConfiguration( ).getConfiguration( ); if ( organisationInformation != null ) { diff --git a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java index 9bb9ed443..e597de438 100644 --- a/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java +++ b/archiva-modules/archiva-base/archiva-repository-admin/archiva-repository-admin-default/src/test/java/org/apache/archiva/admin/repository/admin/ArchivaAdministrationTest.java @@ -222,7 +222,7 @@ public class ArchivaAdministrationTest try { OrganisationInformation newOrganisationInformation = new OrganisationInformation( ); - newOrganisationInformation.setLogoLocation( "'/>" ); + newOrganisationInformation.setLogoLocation( "http://www.foo.com'/>" ); newOrganisationInformation.setName( "foo org" ); newOrganisationInformation.setUrl( "http://foo.com" ); archivaAdministration.setOrganisationInformation( newOrganisationInformation ); @@ -240,7 +240,7 @@ public class ArchivaAdministrationTest try { OrganisationInformation newOrganisationInformation = new OrganisationInformation( ); - newOrganisationInformation.setUrl( "'/>" ); + newOrganisationInformation.setUrl( "http://foo.com'/>" ); newOrganisationInformation.setName( "foo org" ); newOrganisationInformation.setLogoLocation( "http://foo.com/bar.png" ); archivaAdministration.setOrganisationInformation( newOrganisationInformation ); diff --git a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java index f13cd46dd..7791f34cd 100644 --- a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java +++ b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/main/java/org/apache/archiva/rest/services/DefaultArchivaAdministrationService.java @@ -20,11 +20,7 @@ package org.apache.archiva.rest.services; import org.apache.archiva.admin.model.RepositoryAdminException; import org.apache.archiva.admin.model.admin.ArchivaAdministration; -import org.apache.archiva.admin.model.beans.FileType; -import org.apache.archiva.admin.model.beans.LegacyArtifactPath; -import org.apache.archiva.admin.model.beans.NetworkConfiguration; -import org.apache.archiva.admin.model.beans.OrganisationInformation; -import org.apache.archiva.admin.model.beans.UiConfiguration; +import org.apache.archiva.admin.model.beans.*; import org.apache.archiva.repository.scanner.RepositoryContentConsumers; import org.apache.archiva.rest.api.model.AdminRepositoryConsumer; import org.apache.archiva.rest.api.services.ArchivaAdministrationService; @@ -319,7 +315,7 @@ public class DefaultArchivaAdministrationService } catch ( RepositoryAdminException e ) { - throw new ArchivaRestServiceException( e.getMessage(), e ); + throw new ArchivaRestServiceException( e.getMessage(), 400, e ); } } diff --git a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java index c199838f2..bb5241d6a 100644 --- a/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java +++ b/archiva-modules/archiva-web/archiva-rest/archiva-rest-services/src/test/java/org/apache/archiva/rest/services/ArchivaAdministrationServiceTest.java @@ -22,9 +22,11 @@ import org.apache.archiva.admin.model.beans.FileType; import org.apache.archiva.admin.model.beans.OrganisationInformation; import org.apache.archiva.admin.model.beans.UiConfiguration; import org.apache.archiva.rest.api.model.AdminRepositoryConsumer; +import org.apache.archiva.rest.api.services.ArchivaRestServiceException; import org.apache.commons.lang.StringUtils; import org.junit.Test; +import javax.ws.rs.BadRequestException; import java.util.Arrays; import java.util.List; @@ -91,6 +93,53 @@ public class ArchivaAdministrationServiceTest assertEquals( "http://foo.com", organisationInformation.getUrl() ); } + @Test + public void badOrganizationLogoLocation() + throws Exception + { + OrganisationInformation organisationInformation = + getArchivaAdministrationService().getOrganisationInformation(); + + // rest return an empty bean + assertNotNull( organisationInformation ); + organisationInformation = new OrganisationInformation(); + organisationInformation.setLogoLocation( "http://foo.com'/>" ); + organisationInformation.setName( "foo org" ); + organisationInformation.setUrl( "http://foo.com" ); + + try { + getArchivaAdministrationService().setOrganisationInformation(organisationInformation); + fail("RepositoryAdminException expected. Bad URL content should not be allowed for logo location."); + } catch (BadRequestException e) { + // OK + } + + } + + @Test + public void badOrganizationUrl() + throws Exception + { + OrganisationInformation organisationInformation = + getArchivaAdministrationService().getOrganisationInformation(); + + // rest return an empty bean + assertNotNull( organisationInformation ); + + organisationInformation = new OrganisationInformation(); + organisationInformation.setLogoLocation( "http://foo.com/logo.jpg" ); + organisationInformation.setName( "foo org" ); + organisationInformation.setUrl( "http://foo.com'/>" ); + + try { + getArchivaAdministrationService().setOrganisationInformation(organisationInformation); + fail("RepositoryAdminException expected. Bad URL content should not be allowed for logo location."); + } catch (BadRequestException e) { + // OK + } + + } + @Test public void uiConfigurationReadUpdate() throws Exception -- 2.39.5