Skip to content

Commit

Permalink
Upgrade jackson-databind to 2.12.7 (apache#14770)
Browse files Browse the repository at this point in the history
The current version of jackson-databind is flagged for vulnerabilities CVE-2020-28491 (Although cbor format is not used in druid), CVE-2020-36518 (Seems genuine as deeply nested json in can cause resource exhaustion). Updating the dependency to the latest version 2.12.7 to fix these vulnerabilities.
  • Loading branch information
tejaswini-imply authored Aug 9, 2023
1 parent cd817fc commit 550a66d
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 56 deletions.
3 changes: 0 additions & 3 deletions extensions-contrib/compressed-bigdecimal/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@
<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
<version>2.10.5</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand All @@ -138,12 +137,10 @@
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
<version>2.10.2</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
<version>2.10.2</version>
</dependency>
</dependencies>
</project>
5 changes: 5 additions & 0 deletions extensions-contrib/kafka-emitter/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
<artifactId>jackson-core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-joda</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.apache.druid.emitter.kafka;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.joda.JodaModule;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.apache.druid.java.util.common.DateTimes;
Expand Down Expand Up @@ -102,9 +103,11 @@ public void testKafkaEmitter() throws InterruptedException
requestTopic == null ? totalEventsExcludingRequestLogEvents : totalEvents);

final KafkaProducer<String, String> producer = mock(KafkaProducer.class);
ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new JodaModule());
final KafkaEmitter kafkaEmitter = new KafkaEmitter(
new KafkaEmitterConfig("", eventsType, "metrics", "alerts", requestTopic, "metadata", "test-cluster", null),
new ObjectMapper()
mapper
)
{
@Override
Expand Down
6 changes: 3 additions & 3 deletions licenses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ name: Jackson
license_category: binary
module: java-core
license_name: Apache License version 2.0
version: 2.10.5
version: 2.12.7
libraries:
- com.fasterxml.jackson.core: jackson-annotations
- com.fasterxml.jackson.core: jackson-core
Expand Down Expand Up @@ -289,7 +289,7 @@ name: Jackson
license_category: binary
module: java-core
license_name: Apache License version 2.0
version: 2.10.5.1
version: 2.12.7
libraries:
- com.fasterxml.jackson.core: jackson-databind
notice: |
Expand Down Expand Up @@ -3592,7 +3592,7 @@ libraries:
---

name: Jackson Dataformat Yaml
version: 2.10.5
version: 2.12.7
license_category: binary
module: extensions/druid-avro-extensions
license_name: Apache License version 2.0
Expand Down
16 changes: 0 additions & 16 deletions owasp-dependency-check-suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,6 @@
-->
<cve>CVE-2022-42003</cve>
<cve>CVE-2022-42004</cve>
<!-- CVE-2021-46877 only applies to jdk serialization which we do not use
https://nvd.nist.gov/vuln/detail/CVE-2021-46877
https://github.com/FasterXML/jackson-databind/issues/3328
-->
<cve>CVE-2021-46877</cve>
<!-- According to jackson community, this is not a security issue, see https://github.com/FasterXML/jackson-databind/issues/3972#issuecomment-1596193098, https://github.com/jeremylong/DependencyCheck/issues/5779 -->
<cve>CVE-2023-35116</cve>
</suppress>


Expand Down Expand Up @@ -638,7 +631,6 @@
<notes><![CDATA[
file name: *jackson-*.jar
]]></notes>
<cve>CVE-2020-36518</cve>
<cve>CVE-2022-45688</cve>
</suppress>

Expand Down Expand Up @@ -675,14 +667,6 @@
<cve>CVE-2020-17516</cve>
</suppress>

<suppress>
<!-- jackson-dataformat-cbor-2.10.5.jar -->
<notes><![CDATA[
file name: jackson-dataformat-cbor-2.10.5.jar
]]></notes>
<cve>CVE-2020-28491</cve>
</suppress>

<suppress>
<!-- okhttp -->
<notes><![CDATA[
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
<hamcrest.version>1.3</hamcrest.version>
<jetty.version>9.4.51.v20230217</jetty.version>
<jersey.version>1.19.4</jersey.version>
<jackson.version>2.10.5.20201202</jackson.version>
<jackson.version>2.12.7</jackson.version>
<codehaus.jackson.version>1.9.13</codehaus.jackson.version>
<log4j.version>2.18.0</log4j.version>
<mysql.version>5.1.49</mysql.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.introspect.Annotated;
import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
import com.fasterxml.jackson.databind.introspect.AnnotatedField;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;
Expand All @@ -33,19 +34,31 @@
import org.apache.druid.java.util.common.IAE;

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Map;

/**
*/
public class GuiceAnnotationIntrospector extends NopAnnotationIntrospector
{
@Override
public Object findInjectableValueId(AnnotatedMember m)
public JacksonInject.Value findInjectableValue(AnnotatedMember m)
{
Object id = findGuiceInjectId(m);
if (id == null) {
return null;
}
return JacksonInject.Value.forId(id);
}

private Object findGuiceInjectId(AnnotatedMember m)
{
if (m.getAnnotation(JacksonInject.class) == null) {
return null;
}

Type genericType = null;

Annotation guiceAnnotation = null;
for (Annotation annotation : m.annotations()) {
if (annotation.annotationType().isAnnotationPresent(BindingAnnotation.class)) {
Expand All @@ -54,39 +67,54 @@ public Object findInjectableValueId(AnnotatedMember m)
}
}

// Annotated.getGenericType() is removed since jackson-databind 2.11 version. We need the generic type so that we can inject values of the type such as List<XYZ> correctly.
// Jackson library removed the method on the abstract class but the methods are still there in the implementations of AnnotatedMember. The method signatures are implementation specific and we are calling those methods to get the generic type.

if (m instanceof AnnotatedField) {
genericType = ((AnnotatedField) m).getAnnotated().getGenericType();
} else if (m instanceof AnnotatedMethod) {
genericType = ((AnnotatedMethod) m).getAnnotated().getGenericReturnType();
} else if (m instanceof AnnotatedParameter) {
genericType = ((AnnotatedParameter) m).getOwner().getGenericParameterType(((AnnotatedParameter) m).getIndex());
}

if (genericType == null) {
// Fall back to type-erased raw type in case we missed an implementation. We are unlikely to ever get here though
genericType = m.getRawType();
}
if (guiceAnnotation == null) {
if (m instanceof AnnotatedMethod) {
throw new IAE("Annotated methods don't work very well yet...");
}
return Key.get(m.getGenericType());
return Key.get(genericType);
}
return Key.get(m.getGenericType(), guiceAnnotation);
return Key.get(genericType, guiceAnnotation);
}

/**
* This method is used to find what property to ignore in deserialization. Jackson calls this method
* per every class and every constructor parameter.
*
* This implementation returns a {@link JsonIgnoreProperties.Value#empty()} that allows empty names if
* the parameters has the {@link JsonProperty} annotation. Otherwise, it returns
* {@code JsonIgnoreProperties.Value.forIgnoredProperties("")} that does NOT allow empty names.
* This behavior is to work around a bug in Jackson deserializer (see the below comment for details) and
* can be removed in the future after the bug is fixed.
* For example, suppose a constructor like below:
*
* <pre>{@code
* @JsonCreator
* public ClassWithJacksonInject(
* @JsonProperty("val") String val,
* @JacksonInject InjectedParameter injected
* )
* }</pre>
*
* During deserializing a JSON string into this class, this method will be called at least twice,
* one for {@code val} and another for {@code injected}. It will return {@code Value.empty()} for {@code val},
* while {Value.forIgnoredProperties("")} for {@code injected} because the later does not have {@code JsonProperty}.
* As a result, {@code injected} will be ignored during deserialization since it has no name.
*/
/**
* This method is used to find what property to ignore in deserialization. Jackson calls this method
* per every class and every constructor parameter.
*
* This implementation returns a {@link JsonIgnoreProperties.Value#empty()} that allows empty names if
* the parameters has the {@link JsonProperty} annotation. Otherwise, it returns
* {@code JsonIgnoreProperties.Value.forIgnoredProperties("")} that does NOT allow empty names.
* This behavior is to work around a bug in Jackson deserializer (see the below comment for details) and
* can be removed in the future after the bug is fixed.
* For example, suppose a constructor like below:
*
* <pre>{@code
* @JsonCreator
* public ClassWithJacksonInject(
* @JsonProperty("val") String val,
* @JacksonInject InjectedParameter injected
* )
* }</pre>
*
* During deserializing a JSON string into this class, this method will be called at least twice,
* one for {@code val} and another for {@code injected}. It will return {@code Value.empty()} for {@code val},
* while {Value.forIgnoredProperties("")} for {@code injected} because the later does not have {@code JsonProperty}.
* As a result, {@code injected} will be ignored during deserialization since it has no name.
*/
@Override
public JsonIgnoreProperties.Value findPropertyIgnorals(Annotated ac)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.segment;

import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.databind.AnnotationIntrospector;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -85,7 +86,7 @@ public static AnnotationIntrospector makeAnnotationIntrospector()
return new GuiceAnnotationIntrospector()
{
@Override
public Object findInjectableValueId(AnnotatedMember m)
public JacksonInject.Value findInjectableValue(AnnotatedMember m)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,22 @@ public void testValidationInsaneError()
throw new IllegalStateException("Should have already failed");
}

@Test(expected = ProvisionException.class)
@Test
public void testTRUE()
{
properties.put(PROPERTY_PREFIX + ".populateCache", "TRUE");
configProvider.inject(properties, configurator);
CacheConfig config = configProvider.get();
throw new IllegalStateException("Should have already failed");
Assert.assertTrue(config.isPopulateCache());
}

@Test(expected = ProvisionException.class)
@Test
public void testFALSE()
{
properties.put(PROPERTY_PREFIX + ".populateCache", "FALSE");
configProvider.inject(properties, configurator);
CacheConfig config = configProvider.get();
throw new IllegalStateException("Should have already failed");
Assert.assertFalse(config.isPopulateCache());
}


Expand Down

0 comments on commit 550a66d

Please sign in to comment.