CVE-2018-6331: Deserialization of Untrusted Data (CWE-502)
value: Buck parser-cache command loads/saves state using Java serialized object. If the state information is maliciously crafted, deserializing it could lead to code execution. This issue affects Buck versions prior to v2018.06.25.01.
CWE description: The product deserializes untrusted data without sufficiently verifying that the resulting data will be valid.
CVSS v3 base score: 9.8
Published: 2018-12-31T23:29Z
Repository: https://github.com/facebook/buck
Language: Java
Files changed: src/com/facebook/buck/cli/ParserCacheCommand.java, src/com/facebook/buck/parser/BUCK, src/com/facebook/buck/parser/ParserStateObjectInputStream.java, test/com/facebook/buck/cli/ParserCacheCommandIntegrationTest.java
Commit date: 2018-03-28 12:14:47 -0700
Commit message: Prevent deserialization of random objects

Summary:
Fixed security issue with buck parser-cache command.
This diff prevent deserialization of random objects.

Reviewed By: jtorkkola

fbshipit-source-id: 24e8221

Diff: diff --git a/src/com/facebook/buck/cli/ParserCacheCommand.java b/src/com/facebook/buck/cli/ParserCacheCommand.java
index e448573761..bf7fde1086 100644
--- a/src/com/facebook/buck/cli/ParserCacheCommand.java
+++ b/src/com/facebook/buck/cli/ParserCacheCommand.java
@@ -15,10 +15,11 @@
  */
 
 package com.facebook.buck.cli;
 
 import com.facebook.buck.parser.ParserConfig;
+import com.facebook.buck.parser.ParserStateObjectInputStream;
 import com.facebook.buck.parser.thrift.RemoteDaemonicParserState;
 import com.facebook.buck.util.ExitCode;
 import com.facebook.buck.util.json.ObjectMappers;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.base.Preconditions;
@@ -86,11 +87,11 @@ public class ParserCacheCommand extends AbstractCommand {
     } else if (loadFilename != null) {
       try (FileInputStream fis = new FileInputStream(loadFilename);
           ZipInputStream zipis = new ZipInputStream(fis)) {
         ZipEntry entry = zipis.getNextEntry();
         Preconditions.checkState(entry.getName().equals("parser_data"));
-        try (ObjectInputStream ois = new ObjectInputStream(zipis)) {
+        try (ObjectInputStream ois = new ParserStateObjectInputStream(zipis)) {
           RemoteDaemonicParserState state;
           try {
             state = (RemoteDaemonicParserState) ois.readObject();
           } catch (ClassNotFoundException e) {
             params.getConsole().printErrorText("Invalid file format");
diff --git a/src/com/facebook/buck/parser/BUCK b/src/com/facebook/buck/parser/BUCK
index b7c45ed0fd..f3419fa498 100644
--- a/src/com/facebook/buck/parser/BUCK
+++ b/src/com/facebook/buck/parser/BUCK
@@ -33,10 +33,11 @@ java_immutables_library(
         "ParseEvent.java",
         "ParsePipeline.java",
         "Parser.java",
         "ParserMessages.java",
         "ParserPythonInterpreterProvider.java",
+        "ParserStateObjectInputStream.java",
         "ParserTargetNodeFactory.java",
         "PerBuildState.java",
         "PipelineNodeCache.java",
         "ProjectBuildFileParserFactory.java",
         "ProjectBuildFileParserPool.java",
diff --git a/src/com/facebook/buck/parser/ParserStateObjectInputStream.java b/src/com/facebook/buck/parser/ParserStateObjectInputStream.java
new file mode 100644
index 0000000000..6cd7f2ebc3
--- /dev/null
+++ b/src/com/facebook/buck/parser/ParserStateObjectInputStream.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2013-present Facebook, Inc.
+ *
+ * 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 com.facebook.buck.parser;
+
+import com.facebook.buck.parser.thrift.RemoteDaemonicParserState;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InvalidClassException;
+import java.io.ObjectInputStream;
+import java.io.ObjectStreamClass;
+import java.util.HashSet;
+import java.util.Set;
+
+/** A ObjectInputStream that will deserialize only RemoteDaemonicParserState. */
+public class ParserStateObjectInputStream extends ObjectInputStream {
+
+  private Set<String> whitelist;
+
+  public ParserStateObjectInputStream(InputStream inputStream) throws IOException {
+    super(inputStream);
+
+    whitelist = new HashSet<>();
+    whitelist.add(RemoteDaemonicParserState.class.getName());
+  }
+
+  @Override
+  protected Class<?> resolveClass(ObjectStreamClass desc)
+      throws IOException, ClassNotFoundException {
+    if (!whitelist.contains(desc.getName())) {
+      throw new InvalidClassException(desc.getName(), "Can't deserialize this class");
+    }
+    return super.resolveClass(desc);
+  }
+}
diff --git a/test/com/facebook/buck/cli/ParserCacheCommandIntegrationTest.java b/test/com/facebook/buck/cli/ParserCacheCommandIntegrationTest.java
index 50a9ed9e72..0de3e511a1 100644
--- a/test/com/facebook/buck/cli/ParserCacheCommandIntegrationTest.java
+++ b/test/com/facebook/buck/cli/ParserCacheCommandIntegrationTest.java
@@ -26,21 +26,30 @@ import com.facebook.buck.testutil.TemporaryPaths;
 import com.facebook.buck.testutil.integration.ProjectWorkspace;
 import com.facebook.buck.testutil.integration.TestContext;
 import com.facebook.buck.testutil.integration.TestDataHelper;
 import com.facebook.buck.util.HumanReadableException;
 import com.facebook.buck.util.NamedTemporaryFile;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectOutputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 import org.hamcrest.Matchers;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class ParserCacheCommandIntegrationTest {
 
   @Rule public TemporaryPaths tmp = new TemporaryPaths();
+  @Rule public ExpectedException thrown = ExpectedException.none();
 
   @Test
   public void testSaveAndLoad() throws IOException {
     assumeTrue(AppleNativeIntegrationTestUtils.isApplePlatformAvailable(ApplePlatform.MACOSX));
 
@@ -144,6 +153,30 @@ public class ParserCacheCommandIntegrationTest {
     } catch (HumanReadableException e) {
       assertThat(
           e.getMessage(), Matchers.containsString("//Apps:TestAppsLibrary could not be found"));
     }
   }
+
+  @Test
+  public void testInvalidData() throws IOException {
+    Map<String, String> invalidData = new HashMap();
+    invalidData.put("foo", "bar");
+
+    NamedTemporaryFile tempFile = new NamedTemporaryFile("invalid_parser_data", null);
+    try (FileOutputStream fos = new FileOutputStream(tempFile.get().toString());
+        ZipOutputStream zipos = new ZipOutputStream(fos)) {
+      zipos.putNextEntry(new ZipEntry("parser_data"));
+      try (ObjectOutputStream oos = new ObjectOutputStream(zipos)) {
+        oos.writeObject(invalidData);
+      }
+    }
+
+    ProjectWorkspace workspace =
+        TestDataHelper.createProjectWorkspaceForScenario(this, "parser_with_cell", tmp);
+    workspace.setUp();
+
+    // Load the invalid parser cache data.
+    thrown.expect(InvalidClassException.class);
+    thrown.expectMessage("Can't deserialize this class");
+    workspace.runBuckCommand("parser-cache", "--load", tempFile.get().toString());
+  }
 }

Vulnerable code:         try (ObjectInputStream ois = new ObjectInputStream(zipis)) {
Fixed code: import com.facebook.buck.parser.ParserStateObjectInputStream;
        try (ObjectInputStream ois = new ParserStateObjectInputStream(zipis)) {
        "ParserStateObjectInputStream.java",
/*
 * Copyright 2013-present Facebook, Inc.
 *
 * 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 com.facebook.buck.parser;

import com.facebook.buck.parser.thrift.RemoteDaemonicParserState;
import java.io.IOException;
import java.io.InputStream;
import java.io.InvalidClassException;
import java.io.ObjectInputStream;
import java.io.ObjectStreamClass;
import java.util.HashSet;
import java.util.Set;

/** A ObjectInputStream that will deserialize only RemoteDaemonicParserState. */
public class ParserStateObjectInputStream extends ObjectInputStream {

  private Set<String> whitelist;

  public ParserStateObjectInputStream(InputStream inputStream) throws IOException {
    super(inputStream);

    whitelist = new HashSet<>();
    whitelist.add(RemoteDaemonicParserState.class.getName());
  }

  @Override
  protected Class<?> resolveClass(ObjectStreamClass desc)
      throws IOException, ClassNotFoundException {
    if (!whitelist.contains(desc.getName())) {
      throw new InvalidClassException(desc.getName(), "Can't deserialize this class");
    }
    return super.resolveClass(desc);
  }
}
import java.io.FileOutputStream;
import java.io.InvalidClassException;
import java.io.ObjectOutputStream;
import java.util.HashMap;
import java.util.Map;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.junit.rules.ExpectedException;
  @Rule public ExpectedException thrown = ExpectedException.none();

  @Test
  public void testInvalidData() throws IOException {
    Map<String, String> invalidData = new HashMap();
    invalidData.put("foo", "bar");

    NamedTemporaryFile tempFile = new NamedTemporaryFile("invalid_parser_data", null);
    try (FileOutputStream fos = new FileOutputStream(tempFile.get().toString());
        ZipOutputStream zipos = new ZipOutputStream(fos)) {
      zipos.putNextEntry(new ZipEntry("parser_data"));
      try (ObjectOutputStream oos = new ObjectOutputStream(zipos)) {
        oos.writeObject(invalidData);
      }
    }

    ProjectWorkspace workspace =
        TestDataHelper.createProjectWorkspaceForScenario(this, "parser_with_cell", tmp);
    workspace.setUp();

    // Load the invalid parser cache data.
    thrown.expect(InvalidClassException.class);
    thrown.expectMessage("Can't deserialize this class");
    workspace.runBuckCommand("parser-cache", "--load", tempFile.get().toString());
  }