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 8a13d818fe05aa13055fc1208da4764be109f872 Author: Martin Stockhammer <[email protected]> AuthorDate: Sat Apr 13 11:59:29 2019 +0200 [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 +--- .../services/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 0c8a682..3be9f58 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 9bb9ed4..e597de4 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( "'/><svg/onload=alert(/logoLocation_xss/)>" ); + newOrganisationInformation.setLogoLocation( "http://www.foo.com'/><svg/onload=alert(/logoLocation_xss/)>" ); 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( "'/><svg/onload=alert(/url_xss/)>" ); + newOrganisationInformation.setUrl( "http://foo.com'/><svg/onload=alert(/url_xss/)>" ); 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 f13cd46..7791f34 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 c199838..bb5241d 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; @@ -92,6 +94,53 @@ public class ArchivaAdministrationServiceTest } @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'/><svg/onload=alert(/logoLocation_xss/)>" ); + 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'/><svg/onload=alert(/url_xss/)>" ); + + 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 {
![http://foo.com/bar.png"](http://foo.com/bar.png"){kind=link}
![http://foo.com/logo.jpg"](http://foo.com/logo.jpg"){kind=link}