Improving URL check for organisation info

This commit is contained in:
Martin Stockhammer 2019-04-13 11:59:29 +02:00
parent 0dea2b64c3
commit 796716d441
4 changed files with 74 additions and 22 deletions

View File

@ -21,20 +21,14 @@
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.PoolingClientConnectionManager;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.apache.maven.wagon.providers.http.HttpWagon;
import org.springframework.stereotype.Service;
@ -42,10 +36,8 @@
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;
@ -328,14 +320,21 @@ public OrganisationInformation getOrganisationInformation()
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;
}
@ -347,8 +346,9 @@ private String convertName(String name) {
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 )
{

View File

@ -222,7 +222,7 @@ public void badOrganisationInfoLogoLocation( )
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 void badOrganisationInfoUrl( )
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 );

View File

@ -20,11 +20,7 @@
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 void setOrganisationInformation( OrganisationInformation organisationInfo
}
catch ( RepositoryAdminException e )
{
throw new ArchivaRestServiceException( e.getMessage(), e );
throw new ArchivaRestServiceException( e.getMessage(), 400, e );
}
}

View File

@ -23,9 +23,11 @@
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,60 @@ public void organisationInformationUpdate()
assertEquals( "http://foo.com", organisationInformation.getUrl() );
}
@Test
public void badOrganizationLogoLocation()
throws Exception
{
OrganisationInformation organisationInformation =
getArchivaAdministrationService().getOrganisationInformation();
// rest return an empty bean
assertNotNull( organisationInformation );
assertTrue( StringUtils.isBlank( organisationInformation.getLogoLocation() ) );
assertTrue( StringUtils.isBlank( organisationInformation.getName() ) );
assertTrue( StringUtils.isBlank( organisationInformation.getUrl() ) );
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 );
assertTrue( StringUtils.isBlank( organisationInformation.getLogoLocation() ) );
assertTrue( StringUtils.isBlank( organisationInformation.getName() ) );
assertTrue( StringUtils.isBlank( organisationInformation.getUrl() ) );
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