From 5dee8534cd1b92952d10cc56335b5d5856f48f3b Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 31 May 2017 12:19:44 -0500 Subject: [PATCH] Update SecurityJackson2Modules Fixes gh-4370 --- .../jackson2/SecurityJackson2Modules.java | 122 ++++++++++++++++- .../SecurityJackson2ModulesTests.java | 123 ++++++++++++++++++ 2 files changed, 238 insertions(+), 7 deletions(-) create mode 100644 core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java diff --git a/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java b/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java index e97482eeaf..79e8afdd96 100644 --- a/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java +++ b/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java @@ -16,17 +16,18 @@ package org.springframework.security.jackson2; +import com.fasterxml.jackson.annotation.JacksonAnnotation; import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.fasterxml.jackson.databind.Module; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder; +import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.jsontype.*; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.ClassUtils; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; +import java.io.IOException; +import java.util.*; /** * This utility class will find all the SecurityModules in classpath. @@ -65,7 +66,7 @@ public final class SecurityJackson2Modules { if(mapper != null) { TypeResolverBuilder typeBuilder = mapper.getDeserializationConfig().getDefaultTyper(null); if (typeBuilder == null) { - mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); + mapper.setDefaultTyping(createWhitelistedDefaultTyping()); } } } @@ -103,4 +104,111 @@ public final class SecurityJackson2Modules { } return modules; } + + /** + * Creates a TypeResolverBuilder that performs whitelisting. + * @return a TypeResolverBuilder that performs whitelisting. + */ + private static TypeResolverBuilder createWhitelistedDefaultTyping() { + TypeResolverBuilder result = new WhitelistTypeResolverBuilder(ObjectMapper.DefaultTyping.NON_FINAL); + result = result.init(JsonTypeInfo.Id.CLASS, null); + result = result.inclusion(JsonTypeInfo.As.PROPERTY); + return result; + } + + /** + * An implementation of {@link ObjectMapper.DefaultTypeResolverBuilder} that overrides the {@link TypeIdResolver} + * with {@link WhitelistTypeIdResolver}. + * @author Rob Winch + */ + static class WhitelistTypeResolverBuilder extends ObjectMapper.DefaultTypeResolverBuilder { + + public WhitelistTypeResolverBuilder(ObjectMapper.DefaultTyping defaultTyping) { + super(defaultTyping); + } + + protected TypeIdResolver idResolver(MapperConfig config, + JavaType baseType, Collection subtypes, boolean forSer, boolean forDeser) { + TypeIdResolver result = super.idResolver(config, baseType, subtypes, forSer, forDeser); + return new WhitelistTypeIdResolver(result); + } + } + + /** + * A {@link TypeIdResolver} that delegates to an existing implementation and throws an IllegalStateException if the + * class being looked up is not whitelisted, does not provide an explicit mixin, and is not annotated with Jackson + * mappings. See https://github.com/spring-projects/spring-security/issues/4370 + */ + static class WhitelistTypeIdResolver implements TypeIdResolver { + private static final Set WHITELIST_CLASS_NAMES = Collections.unmodifiableSet(new HashSet(Arrays.asList( + "java.util.ArrayList", + "java.util.Collections$EmptyMap", + "java.util.Date", + "java.util.TreeMap", + "org.springframework.security.core.context.SecurityContextImpl" + ))); + + private final TypeIdResolver delegate; + + WhitelistTypeIdResolver(TypeIdResolver delegate) { + this.delegate = delegate; + } + + @Override + public void init(JavaType baseType) { + delegate.init(baseType); + } + + @Override + public String idFromValue(Object value) { + return delegate.idFromValue(value); + } + + @Override + public String idFromValueAndType(Object value, Class suggestedType) { + return delegate.idFromValueAndType(value, suggestedType); + } + + @Override + public String idFromBaseType() { + return delegate.idFromBaseType(); + } + + @Override + public JavaType typeFromId(DatabindContext context, String id) throws IOException { + DeserializationConfig config = (DeserializationConfig) context.getConfig(); + JavaType result = delegate.typeFromId(context, id); + String className = result.getRawClass().getName(); + if(isWhitelisted(className)) { + return delegate.typeFromId(context, id); + } + boolean isExplicitMixin = config.findMixInClassFor(result.getRawClass()) != null; + if(isExplicitMixin) { + return result; + } + JacksonAnnotation jacksonAnnotation = AnnotationUtils.findAnnotation(result.getRawClass(), JacksonAnnotation.class); + if(jacksonAnnotation != null) { + return result; + } + throw new IllegalArgumentException("The class with " + id + " and name of " + className + " is not whitelisted. " + + "If you believe this class is safe to deserialize, please provide an explicit mapping using Jackson annotations or by providing a Mixin. " + + "If the serialization is only done by a trusted source, you can also enable default typing. " + + "See https://github.com/spring-projects/spring-security/issues/4370 for details"); + } + + private boolean isWhitelisted(String id) { + return WHITELIST_CLASS_NAMES.contains(id); + } + + @Override + public String getDescForKnownTypeIds() { + return delegate.getDescForKnownTypeIds(); + } + + @Override + public JsonTypeInfo.Id getMechanism() { + return delegate.getMechanism(); + } + + } } diff --git a/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java b/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java new file mode 100644 index 0000000000..17fb8744e8 --- /dev/null +++ b/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java @@ -0,0 +1,123 @@ +/* + * Copyright 2015-2017 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.security.jackson2; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonIgnoreType; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Before; +import org.junit.Test; + +import java.lang.annotation.*; + +import static org.assertj.core.api.Assertions.*; + +/** +* @author Rob Winch +* @since 5.0 +*/ +public class SecurityJackson2ModulesTests { + private ObjectMapper mapper; + + @Before + public void setup() { + mapper = new ObjectMapper(); + SecurityJackson2Modules.enableDefaultTyping(mapper); + } + + @Test + public void readValueWhenNotWhitelistedOrMappedThenThrowsException() throws Exception { + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + assertThatThrownBy(() -> { + mapper.readValue(content, Object.class); + } + ).hasStackTraceContaining("whitelisted"); + } + + @Test + public void readValueWhenExplicitDefaultTypingAfterSecuritySetupThenReadsAsSpecificType() throws Exception { + mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + } + + @Test + public void readValueWhenExplicitDefaultTypingBeforeSecuritySetupThenReadsAsSpecificType() throws Exception { + mapper = new ObjectMapper(); + mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); + SecurityJackson2Modules.enableDefaultTyping(mapper); + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + } + + @Test + public void readValueWhenAnnotatedThenReadsAsSpecificType() throws Exception { + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelistedButAnnotated\",\"property\":\"bar\"}"; + + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelistedButAnnotated.class); + } + + @Test + public void readValueWhenMixinProvidedThenReadsAsSpecificType() throws Exception { + mapper.addMixIn(NotWhitelisted.class, NotWhitelistedMixin.class); + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + } + + @Target({ ElementType.TYPE, ElementType.ANNOTATION_TYPE }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + public @interface NotJacksonAnnotation {} + + @NotJacksonAnnotation + static class NotWhitelisted { + private String property = "bar"; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + } + } + + @JsonIgnoreType(false) + static class NotWhitelistedButAnnotated { + private String property = "bar"; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + } + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY) + @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE, + isGetterVisibility = JsonAutoDetect.Visibility.NONE) + @JsonIgnoreProperties(ignoreUnknown = true) + abstract class NotWhitelistedMixin { + + } +}