restrict XML decoding of History object

Author: Vladimir Kotal

opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java

@@ -32,11 +32,13 @@
 import java.io.BufferedOutputStream;
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.nio.file.NoSuchFileException;
@@ -236,13 +238,24 @@ private static File getCachedFile(File file) throws HistoryException,
         return new File(TandemPath.join(sb.toString(), ".gz"));
     }
 
+    private static XMLDecoder getDecoder(InputStream in) {
+        return new XMLDecoder(in, null, null, new HistoryClassLoader());
+    }
+
+    // for testing
+    static History readCache(String xmlconfig) {
+        final ByteArrayInputStream in = new ByteArrayInputStream(xmlconfig.getBytes());
+        try (XMLDecoder d = getDecoder(in)) {
+            return (History) d.readObject();
+        }
+    }
+
     /**
      * Read history from a file.
      */
-    private static History readCache(File file) throws IOException {
+    static History readCache(File file) throws IOException {
         try (FileInputStream in = new FileInputStream(file);
-            XMLDecoder d = new XMLDecoder(new GZIPInputStream(
-                new BufferedInputStream(in)))) {
+            XMLDecoder d = getDecoder(new GZIPInputStream(new BufferedInputStream(in)))) {
             return (History) d.readObject();
         }
     }

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryClassLoader.java

@@ -0,0 +1,61 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * See LICENSE.txt included in this distribution for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at LICENSE.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ */
+package org.opengrok.indexer.history;
+
+import java.beans.XMLDecoder;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+
+/**
+ * Temporary hack to prevent {@link XMLDecoder} to deserialize other than allowed classes. This tries to prevent
+ * calling of methods on {@link ProcessBuilder} or {@link Runtime} (or similar) which could be used for code execution.
+ */
+public class HistoryClassLoader extends ClassLoader {
+
+    private static final Set<String> allowedClasses = Set.of(
+            ArrayList.class,
+            Collections.class,
+            Date.class,
+            HashMap.class,
+            History.class,
+            HistoryEntry.class,
+            RepositoryInfo.class,
+            String.class,
+            TreeSet.class,
+            XMLDecoder.class
+    ).stream().map(Class::getName).collect(Collectors.toSet());
+
+    @Override
+    public Class<?> loadClass(final String name) throws ClassNotFoundException {
+        if (!allowedClasses.contains(name)) {
+            throw new IllegalAccessError(name + " is not allowed to be used in configuration");
+        }
+
+        return getClass().getClassLoader().loadClass(name);
+    }
+}

opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java

@@ -28,13 +28,15 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.opengrok.indexer.condition.RepositoryInstalled.Type.MERCURIAL;
 import static org.opengrok.indexer.condition.RepositoryInstalled.Type.SCCS;
 import static org.opengrok.indexer.condition.RepositoryInstalled.Type.SUBVERSION;
 import static org.opengrok.indexer.history.MercurialRepositoryTest.runHgCommand;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.file.Paths;
 import java.util.Date;
 import java.util.Iterator;
@@ -47,6 +49,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.EnabledOnOs;
 import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.opengrok.indexer.condition.EnabledForRepository;
 import org.opengrok.indexer.configuration.Filter;
 import org.opengrok.indexer.configuration.IgnoredNames;
@@ -827,4 +831,60 @@ public void testStoreAndTryToGetIgnored() throws Exception {
         retrievedHistory = cache.get(makefile, repo, true);
         assertNotNull(retrievedHistory, "history for Makefile should not be null");
     }
+
+    @ParameterizedTest
+    @ValueSource(strings = {
+            "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                    "<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
+                    "  <object class=\"java.lang.Runtime\" method=\"getRuntime\">\n" +
+                    "    <void method=\"exec\">\n" +
+                    "      <array class=\"java.lang.String\" length=\"2\">\n" +
+                    "        <void index=\"0\">\n" +
+                    "          <string>/usr/bin/nc</string>\n" +
+                    "        </void>\n" +
+                    "        <void index=\"1\">\n" +
+                    "          <string>-l</string>\n" +
+                    "        </void>\n" +
+                    "      </array>\n" +
+                    "    </void>\n" +
+                    "  </object>\n" +
+                    "</java>",
+            "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                    "<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
+                    "  <object class=\"java.lang.ProcessBuilder\">\n" +
+                    "    <array class=\"java.lang.String\" length=\"1\" >\n" +
+                    "      <void index=\"0\"> \n" +
+                    "        <string>/usr/bin/curl https://oracle.com</string>\n" +
+                    "      </void>\n" +
+                    "    </array>\n" +
+                    "    <void method=\"start\"/>\n" +
+                    "  </object>\n" +
+                    "</java>",
+            "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+                    "<java version=\"11.0.8\" class=\"java.beans.XMLDecoder\">\n" +
+                    "  <object class = \"java.io.FileOutputStream\"> \n" +
+                    "    <string>opengrok_test.txt</string>\n" +
+                    "    <method name = \"write\">\n" +
+                    "      <array class=\"byte\" length=\"3\">\n" +
+                    "        <void index=\"0\"><byte>96</byte></void>\n" +
+                    "        <void index=\"1\"><byte>96</byte></void>\n" +
+                    "        <void index=\"2\"><byte>96</byte></void>\n" +
+                    "      </array>\n" +
+                    "    </method>\n" +
+                    "    <method name=\"close\"/>\n" +
+                    "  </object>\n" +
+                    "</java>"
+    })
+    void testDeserializationOfNotWhiteListedClassThrowsError(final String exploit) {
+        assertThrows(IllegalAccessError.class, () -> FileHistoryCache.readCache(exploit));
+    }
+
+    @Test
+    void testReadCacheValid() throws IOException {
+        File testFile = new File(FileHistoryCacheTest.class.getClassLoader().
+                getResource("history/FileHistoryCache.java.gz").getFile());
+        History history = FileHistoryCache.readCache(testFile);
+        assertNotNull(history);
+        assertEquals(30, history.getHistoryEntries().size());
+    }
 }