Jackson: upgrade for useless CVE-2020-36518 (#8201)

Includes fixups: - new builder pattern - Jackson now correctly implements Include.NON_EMPTY for JsonValue, so things like IntegerSpace are not serialized instead of "". Fix a few deserializers as a result. - switch to jakarta activation API - it's the new thing.
batfish · Mar 30, 2022 · 60e31b8 · 60e31b8
1 parent 05d4762
commit 60e31b8
Show file tree

Hide file tree

Showing 21 changed files with 299 additions and 778 deletions.
diff --git a/.trivyignore b/.trivyignore
@@ -1,2 +0,0 @@
-# Unfixed, fix release than cure. See: https://github.com/FasterXML/jackson-databind/issues/2816#issuecomment-1067138541
-CVE-2020-36518

diff --git a/library_deps.bzl b/library_deps.bzl
@@ -3,15 +3,15 @@ load("@rules_jvm_external//:specs.bzl", "maven")
 # See: https://github.com/bazelbuild/rules_jvm_external#exporting-and-consuming-artifacts-from-external-repositories
 BATFISH_MAVEN_ARTIFACTS = [
     "com.carrotsearch:hppc:0.9.1",
-    "com.fasterxml.jackson.core:jackson-annotations:2.11.4",
-    "com.fasterxml.jackson.core:jackson-core:2.11.4",
-    "com.fasterxml.jackson.core:jackson-databind:2.11.4",
-    "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.11.4",
-    "com.fasterxml.jackson.datatype:jackson-datatype-guava:2.11.4",
-    "com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.11.4",
-    "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.11.4",
-    "com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.11.4",
-    "com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.11.4",
+    "com.fasterxml.jackson.core:jackson-annotations:2.13.2",
+    "com.fasterxml.jackson.core:jackson-core:2.13.2",
+    "com.fasterxml.jackson.core:jackson-databind:2.13.2.2",
+    "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.2",
+    "com.fasterxml.jackson.datatype:jackson-datatype-guava:2.13.2",
+    "com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.13.2",
+    "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.13.2",
+    "com.fasterxml.jackson.jaxrs:jackson-jaxrs-base:2.13.2",
+    "com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.13.2",
     "com.github.ben-manes.caffeine:caffeine:2.9.2",
     "com.google.auto.service:auto-service:1.0-rc6",
     "com.google.auto.service:auto-service-annotations:1.0-rc6",
@@ -25,7 +25,7 @@ BATFISH_MAVEN_ARTIFACTS = [
     "commons-cli:commons-cli:1.4",
     "commons-io:commons-io:2.8.0",
     "dk.brics:automaton:1.12-1",
-    "javax.activation:activation:1.1",
+    "jakarta.activation:jakarta.activation-api:1.2.2",
     "javax.annotation:javax.annotation-api:1.3.2",
     "javax.ws.rs:javax.ws.rs-api:2.1.1",
     "javax.xml.bind:jaxb-api:2.3.0",

diff --git a/maven_install.json b/maven_install.json
diff --git a/projects/batfish-common-protocol/BUILD b/projects/batfish-common-protocol/BUILD
@@ -71,6 +71,7 @@ java_library(
     ],
     runtime_deps = [
         "@maven//:commons_beanutils_commons_beanutils",
+        "@maven//:jakarta_activation_jakarta_activation_api",
         "@maven//:org_glassfish_jersey_inject_jersey_hk2",
     ],
     deps = [

diff --git a/projects/batfish-common-protocol/pom.xml b/projects/batfish-common-protocol/pom.xml
@@ -38,7 +38,7 @@
                                     </ignoredUnusedDeclaredDependency>
                                     <ignoredUnusedDeclaredDependency>commons-beanutils:commons-beanutils
                                     </ignoredUnusedDeclaredDependency>
-                                    <ignoredUnusedDeclaredDependency>javax.activation:activation
+                                    <ignoredUnusedDeclaredDependency>jakarta.activation:jakarta.activation-api
                                     </ignoredUnusedDeclaredDependency>
                                     <ignoredUnusedDeclaredDependency>javax.xml.bind:jaxb-api
                                     </ignoredUnusedDeclaredDependency>
@@ -265,8 +265,8 @@
 
         <!-- Runtime dependencies for Jersey. -->
         <dependency>
-            <groupId>javax.activation</groupId>
-            <artifactId>activation</artifactId>
+            <groupId>jakarta.activation</groupId>
+            <artifactId>jakarta.activation-api</artifactId>
             <scope>runtime</scope>
         </dependency>
 

diff --git a/...ts/batfish-common-protocol/src/main/java/org/batfish/common/util/BatfishObjectMapper.java b/...ts/batfish-common-protocol/src/main/java/org/batfish/common/util/BatfishObjectMapper.java
@@ -11,6 +11,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.fasterxml.jackson.datatype.guava.GuavaModule;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
@@ -19,23 +20,23 @@
 import org.batfish.common.util.serialization.BatfishThirdPartySerializationModule;
 
 public final class BatfishObjectMapper {
-  private static final ObjectMapper MAPPER = baseMapper();
+  private static final JsonMapper MAPPER = baseMapper().build();
 
-  private static final ObjectMapper IGNORE_UNKNOWN_MAPPER =
-      baseMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
+  private static final JsonMapper IGNORE_UNKNOWN_MAPPER =
+      baseMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false).build();
 
   private static final ObjectWriter ALWAYS_WRITER =
-      baseMapper().setSerializationInclusion(Include.ALWAYS).writer();
+      baseMapper().serializationInclusion(Include.ALWAYS).build().writer();
 
   private static final ObjectWriter WRITER = MAPPER.writer();
 
   private static final PrettyPrinter PRETTY_PRINTER = new PrettyPrinter();
 
   private static final ObjectWriter PRETTY_WRITER =
-      baseMapper().enable(SerializationFeature.INDENT_OUTPUT).writer(PRETTY_PRINTER);
+      baseMapper().enable(SerializationFeature.INDENT_OUTPUT).build().writer(PRETTY_PRINTER);
 
   private static final ObjectMapper VERBOSE_MAPPER =
-      baseMapper().setSerializationInclusion(Include.ALWAYS);
+      baseMapper().serializationInclusion(Include.ALWAYS).build();
 
   private static final ObjectWriter VERBOSE_WRITER = VERBOSE_MAPPER.writer(PRETTY_PRINTER);
 
@@ -163,28 +164,23 @@ public PrettyPrinter() {
   }
 
   /** Configures all the default options for a Batfish {@link ObjectMapper}. */
-  private static ObjectMapper baseMapper() {
-    ObjectMapper mapper = new ObjectMapper();
-
-    mapper.disable(MapperFeature.AUTO_DETECT_CREATORS);
-    mapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS);
-    mapper.enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY);
-    // Next two lines make Instant class serialize as an RFC-3339 timestamp
-    mapper.registerModule(new JavaTimeModule());
-    mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
-    // This line makes Java 8's Optional type serialize
-    mapper.registerModule(new Jdk8Module());
-    // See https://groups.google.com/forum/#!topic/jackson-user/WfZzlt5C2Ww
-    //  This fixes issues in which non-empty maps with keys with empty values would get omitted
-    //  entirely. See also https://github.com/batfish/batfish/issues/256
-    mapper.setDefaultPropertyInclusion(
-        JsonInclude.Value.construct(Include.NON_EMPTY, Include.ALWAYS));
-    // This line makes Guava collections work with jackson
-    mapper.registerModule(new GuavaModule());
-
-    // Custom (de)serialization for 3rd-party classes
-    mapper.registerModule(new BatfishThirdPartySerializationModule());
-
-    return mapper;
+  private static JsonMapper.Builder baseMapper() {
+    return JsonMapper.builder()
+        .disable(MapperFeature.AUTO_DETECT_CREATORS)
+        .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS)
+        .enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)
+        // Next two lines make Instant class serialize as an RFC-3339 timestamp
+        .addModule(new JavaTimeModule())
+        .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
+        // This line makes Java 8's Optional type serialize
+        .addModule(new Jdk8Module())
+        // See https://groups.google.com/forum/#!topic/jackson-user/WfZzlt5C2Ww
+        // This fixes issues in which non-empty maps with keys with empty values would get
+        // omitted entirely. See also https://github.com/batfish/batfish/issues/256
+        .defaultPropertyInclusion(JsonInclude.Value.construct(Include.NON_EMPTY, Include.ALWAYS))
+        // This line makes Guava collections work with Jackson
+        .addModule(new GuavaModule())
+        // Custom (de)serialization for 3rd-party classes
+        .addModule(new BatfishThirdPartySerializationModule());
   }
 }
diff --git a/projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Interface.java b/projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Interface.java
@@ -926,7 +926,7 @@ private static InterfaceType computeVyosInterfaceType(String name) {
   private boolean _active;
   private @Nonnull IpSpace _additionalArpIps;
   private boolean _adminUp;
-  private IntegerSpace _allowedVlans;
+  private @Nonnull IntegerSpace _allowedVlans;
   @Nonnull private SortedSet<InterfaceAddress> _allAddresses;
   @Nonnull private SortedMap<ConcreteInterfaceAddress, ConnectedRouteMetadata> _addressMetadata;
   /** Cache of all concrete addresses */
@@ -1184,7 +1184,7 @@ public boolean getAdminUp() {
 
   /** Ranges of allowed VLANs when switchport mode is TRUNK. */
   @JsonProperty(PROP_ALLOWED_VLANS)
-  public IntegerSpace getAllowedVlans() {
+  public @Nonnull IntegerSpace getAllowedVlans() {
     return _allowedVlans;
   }
 
@@ -1687,8 +1687,8 @@ private void setAdminUp(boolean adminUp) {
   }
 
   @JsonProperty(PROP_ALLOWED_VLANS)
-  public void setAllowedVlans(IntegerSpace allowedVlans) {
-    _allowedVlans = allowedVlans;
+  public void setAllowedVlans(@Nullable IntegerSpace allowedVlans) {
+    _allowedVlans = firstNonNull(allowedVlans, IntegerSpace.EMPTY);
   }
 
   @JsonProperty(PROP_ALL_PREFIXES)

diff --git a/...n-protocol/src/main/java/org/batfish/datamodel/routing_policy/expr/ExplicitPrefixSet.java b/...n-protocol/src/main/java/org/batfish/datamodel/routing_policy/expr/ExplicitPrefixSet.java
@@ -1,7 +1,10 @@
 package org.batfish.datamodel.routing_policy.expr;
 
+import static com.google.common.base.MoreObjects.firstNonNull;
+
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import javax.annotation.Nullable;
 import org.batfish.datamodel.Prefix;
 import org.batfish.datamodel.PrefixSpace;
 import org.batfish.datamodel.routing_policy.Environment;
@@ -10,10 +13,13 @@
 public class ExplicitPrefixSet extends PrefixSetExpr implements PrefixSpaceExpr {
   private static final String PROP_PREFIX_SPACE = "prefixSpace";
 
-  private PrefixSpace _prefixSpace;
+  private final PrefixSpace _prefixSpace;
 
   @JsonCreator
-  private ExplicitPrefixSet() {}
+  private static ExplicitPrefixSet jsonCreator(
+      @Nullable @JsonProperty(PROP_PREFIX_SPACE) PrefixSpace prefixSpace) {
+    return new ExplicitPrefixSet(firstNonNull(prefixSpace, new PrefixSpace()));
+  }
 
   public ExplicitPrefixSet(PrefixSpace prefixSpace) {
     _prefixSpace = prefixSpace;
@@ -64,9 +70,4 @@ public boolean matches(Prefix prefix, Environment environment) {
     boolean value = _prefixSpace.containsPrefix(prefix);
     return value;
   }
-
-  @JsonProperty(PROP_PREFIX_SPACE)
-  public void setPrefixSpace(PrefixSpace prefixSpace) {
-    _prefixSpace = prefixSpace;
-  }
 }
diff --git a/...ish-common-protocol/src/main/java/org/batfish/datamodel/vendor_family/cumulus/Bridge.java b/...ish-common-protocol/src/main/java/org/batfish/datamodel/vendor_family/cumulus/Bridge.java
@@ -63,9 +63,10 @@ private Builder() {}
       @JsonProperty(PROP_PVID) @Nullable Integer pvid,
       @JsonProperty(PROP_VIDS) @Nullable IntegerSpace vids) {
     checkArgument(pvid != null, "Missing %s", PROP_PVID);
-    checkArgument(vids != null, "Missing %s", PROP_VIDS);
     return new Bridge(
-        ImmutableSortedSet.copyOf(firstNonNull(ports, ImmutableSortedSet.of())), pvid, vids);
+        ImmutableSortedSet.copyOf(firstNonNull(ports, ImmutableSortedSet.of())),
+        pvid,
+        firstNonNull(vids, IntegerSpace.EMPTY));
   }
 
   private final @Nonnull SortedSet<String> _ports;

diff --git a/projects/build-tools/src/main/resources/org/batfish/owasp/suppressions.xml b/projects/build-tools/src/main/resources/org/batfish/owasp/suppressions.xml
@@ -4,8 +4,4 @@
     <notes>This is a SUSE packaging bug, nothing to do with jar.</notes>
     <cve>CVE-2020-8022</cve>
   </suppress>
-  <suppress>
-    <notes>This is a non-issue.</notes>
-    <cve>CVE-2020-36518</cve>
-  </suppress>
 </suppressions>
diff --git a/projects/coordinator/BUILD b/projects/coordinator/BUILD
@@ -12,7 +12,6 @@ java_library(
     resources = glob(["src/main/resources/**"]),
     runtime_deps = [
         "//projects/question",
-        "@maven//:javax_activation_activation",
         "@maven//:javax_xml_bind_jaxb_api",
         "@maven//:org_glassfish_jersey_inject_jersey_hk2",
     ],

diff --git a/projects/coordinator/pom.xml b/projects/coordinator/pom.xml
@@ -27,7 +27,7 @@
                             <configuration>
                                 <ignoredUnusedDeclaredDependencies>
                                     <ignoredUnusedDeclaredDependency>
-                                        javax.activation:activation
+                                        jakarta.activation:jakarta.activation-api
                                     </ignoredUnusedDeclaredDependency>
                                     <ignoredUnusedDeclaredDependency>
                                         javax.xml.bind:jaxb-api
@@ -169,8 +169,8 @@
 
         <!-- Runtime dependencies for Jersey. -->
         <dependency>
-            <groupId>javax.activation</groupId>
-            <artifactId>activation</artifactId>
+            <groupId>jakarta.activation</groupId>
+            <artifactId>jakarta.activation-api</artifactId>
             <scope>runtime</scope>
         </dependency>
 

diff --git a/projects/pom.xml b/projects/pom.xml
@@ -69,8 +69,8 @@
         <hamcrest.version>2.2</hamcrest.version>
         <hppc.version>0.9.1</hppc.version>
         <icu4j.version>69.1</icu4j.version>
-        <jackson.version>2.11.4</jackson.version>
-        <javax-activation.version>1.1</javax-activation.version>
+        <jackson.version>2.13.2.20220328</jackson.version>
+        <jakarta-activation.version>1.2.2</jakarta-activation.version>
         <javax-annotation-api.version>1.3.2</javax-annotation-api.version>
         <jaxb-api.version>2.3.0</jaxb-api.version>
         <jaxrs.version>2.1.1</jaxrs.version>
@@ -750,9 +750,9 @@
             </dependency>
 
             <dependency>
-                <groupId>javax.activation</groupId>
-                <artifactId>activation</artifactId>
-                <version>${javax-activation.version}</version>
+                <groupId>jakarta.activation</groupId>
+                <artifactId>jakarta.activation-api</artifactId>
+                <version>${jakarta-activation.version}</version>
             </dependency>
 
             <dependency>