From 389874b0c822973d5acc270847760c74cca1f684 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Thu, 15 Feb 2018 13:03:27 -0600 Subject: [PATCH] Prevent execution of remote code via mapping of parameters to arbitrary beans. --- ...ncParameterizedServiceInstanceRequest.java | 8 +- .../CreateServiceInstanceBindingRequest.java | 8 +- .../model/ParameterBeanMapper.java | 61 ++++++++++++ .../model/ParameterBeanMapperTest.java | 99 +++++++++++++++++++ 4 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 src/main/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapper.java create mode 100644 src/test/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapperTest.java diff --git a/src/main/java/org/springframework/cloud/servicebroker/model/AsyncParameterizedServiceInstanceRequest.java b/src/main/java/org/springframework/cloud/servicebroker/model/AsyncParameterizedServiceInstanceRequest.java index afcf45af..c80367d0 100644 --- a/src/main/java/org/springframework/cloud/servicebroker/model/AsyncParameterizedServiceInstanceRequest.java +++ b/src/main/java/org/springframework/cloud/servicebroker/model/AsyncParameterizedServiceInstanceRequest.java @@ -39,12 +39,6 @@ public AsyncParameterizedServiceInstanceRequest(Map parameters, } public T getParameters(Class cls) { - try { - T bean = cls.newInstance(); - BeanUtils.populate(bean, parameters); - return bean; - } catch (Exception e) { - throw new IllegalArgumentException("Error mapping parameters to class of type " + cls.getName(), e); - } + return ParameterBeanMapper.mapParametersToBean(parameters, cls); } } diff --git a/src/main/java/org/springframework/cloud/servicebroker/model/CreateServiceInstanceBindingRequest.java b/src/main/java/org/springframework/cloud/servicebroker/model/CreateServiceInstanceBindingRequest.java index 731eaf4a..0ebb66bb 100644 --- a/src/main/java/org/springframework/cloud/servicebroker/model/CreateServiceInstanceBindingRequest.java +++ b/src/main/java/org/springframework/cloud/servicebroker/model/CreateServiceInstanceBindingRequest.java @@ -130,13 +130,7 @@ public CreateServiceInstanceBindingRequest(String serviceDefinitionId, String pl } public T getParameters(Class cls) { - try { - T bean = cls.newInstance(); - BeanUtils.populate(bean, parameters); - return bean; - } catch (Exception e) { - throw new IllegalArgumentException("Error mapping parameters to class of type " + cls.getName()); - } + return ParameterBeanMapper.mapParametersToBean(parameters, cls); } public CreateServiceInstanceBindingRequest withServiceDefinition(final ServiceDefinition serviceDefinition) { diff --git a/src/main/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapper.java b/src/main/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapper.java new file mode 100644 index 00000000..d9746c40 --- /dev/null +++ b/src/main/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapper.java @@ -0,0 +1,61 @@ +/* + * Copyright 2002-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.servicebroker.model; + +import org.apache.commons.beanutils.BeanUtilsBean; +import org.apache.commons.beanutils.SuppressPropertiesBeanIntrospector; + +import java.lang.reflect.InvocationTargetException; +import java.util.Map; + +/** + * Utilities for mapping parameter maps to Java beans. + * + * @author Scott Frederick + */ +public final class ParameterBeanMapper { + /** + * Instantiates an object of the specified type and populates properties of the object from the provided + * parameters. + * + * @param parameters a {@link Map} of values to populate the object from + * @param cls the {@link Class} representing the type of the object to instantiate and populate + * @param the type of the object to instantiate and populate + * @return the instantiated and populated object + */ + public static T mapParametersToBean(Map parameters, Class cls) { + try { + T bean = cls.newInstance(); + + BeanUtilsBean beanUtils = new BeanUtilsBean(); + beanUtils.getPropertyUtils().addBeanIntrospector(SuppressPropertiesBeanIntrospector.SUPPRESS_CLASS); + beanUtils.populate(bean, parameters); + + return bean; + } catch (InstantiationException e) { + throw newIllegalArgumentException(cls, e); + } catch (IllegalAccessException e) { + throw newIllegalArgumentException(cls, e); + } catch (InvocationTargetException e) { + throw newIllegalArgumentException(cls, e); + } + } + + private static IllegalArgumentException newIllegalArgumentException(Class cls, Exception e) { + return new IllegalArgumentException("Error mapping parameters to class of type " + cls.getName(), e); + } +} diff --git a/src/test/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapperTest.java b/src/test/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapperTest.java new file mode 100644 index 00000000..decfd971 --- /dev/null +++ b/src/test/java/org/springframework/cloud/servicebroker/model/ParameterBeanMapperTest.java @@ -0,0 +1,99 @@ +/* + * Copyright 2002-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.servicebroker.model; + +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; + +public class ParameterBeanMapperTest { + @Test + @SuppressWarnings("serial") + public void mapParametersToBean() { + Map parameters = new HashMap() {{ + put("stringProperty", "value1"); + put("intProperty", 2); + put("extraProperty", "extra"); + put("nestedBean.booleanProperty", true); + }}; + TestBean testBean = ParameterBeanMapper.mapParametersToBean(parameters, TestBean.class); + + assertThat(testBean.getStringProperty(), equalTo("value1")); + assertThat(testBean.getIntProperty(), equalTo(2)); + assertThat(testBean.getUnusedProperty(), nullValue()); + assertThat(testBean.getNestedBean().getBooleanProperty(), equalTo(true)); + } + + @SuppressWarnings("unused") + public static final class TestBean { + private String stringProperty; + private int intProperty; + private String unusedProperty; + + private NestedBean nestedBean; + + public TestBean() { + this.nestedBean = new NestedBean(); + } + + public String getStringProperty() { + return stringProperty; + } + + public void setStringProperty(String stringProperty) { + this.stringProperty = stringProperty; + } + + public int getIntProperty() { + return intProperty; + } + + public void setIntProperty(int intProperty) { + this.intProperty = intProperty; + } + + public String getUnusedProperty() { + return unusedProperty; + } + + public void setUnusedProperty(String unusedProperty) { + this.unusedProperty = unusedProperty; + } + + public NestedBean getNestedBean() { + return nestedBean; + } + } + + @SuppressWarnings("unused") + public static final class NestedBean { + private boolean booleanProperty; + + public boolean getBooleanProperty() { + return booleanProperty; + } + + public void setBooleanProperty(boolean booleanProperty) { + this.booleanProperty = booleanProperty; + } + } +} \ No newline at end of file