From d929ea3aae4625117a3fef913184e9dd8c1f4d34 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Wed, 11 Nov 2015 11:36:44 +0000 Subject: [PATCH] #58597: Add more AccessController.doPrivileged. We should fix them later! git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1713813 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/openxml4j/util/ZipSecureFile.java | 34 ++++++++++++------- .../java/org/apache/poi/util/OOXMLLite.java | 22 ++++++++---- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java index 375982b876..5a841ddfc3 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -23,6 +23,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.PushbackInputStream; import java.lang.reflect.Field; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.zip.InflaterInputStream; import java.util.zip.ZipEntry; import java.util.zip.ZipException; @@ -31,6 +33,7 @@ import java.util.zip.ZipInputStream; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; +import org.apache.poi.util.SuppressForbidden; /** * This class wraps a {@link ZipFile} in order to check the @@ -163,20 +166,27 @@ public class ZipSecureFile extends ZipFile { return addThreshold(zipIS); } - @SuppressWarnings("resource") - public static ThresholdInputStream addThreshold(InputStream zipIS) throws IOException { + public static ThresholdInputStream addThreshold(final InputStream zipIS) throws IOException { ThresholdInputStream newInner; if (zipIS instanceof InflaterInputStream) { - try { - Field f = FilterInputStream.class.getDeclaredField("in"); - f.setAccessible(true); - InputStream oldInner = (InputStream)f.get(zipIS); - newInner = new ThresholdInputStream(oldInner, null); - f.set(zipIS, newInner); - } catch (Exception ex) { - logger.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); - newInner = null; - } + newInner = AccessController.doPrivileged(new PrivilegedAction() { + @SuppressForbidden("TODO: Fix this to not use reflection (it will break in Java 9)! " + + "Better would be to wrap *before* instead of tyring to insert wrapper afterwards.") + public ThresholdInputStream run() { + ThresholdInputStream newInner = null; + try { + Field f = FilterInputStream.class.getDeclaredField("in"); + f.setAccessible(true); + InputStream oldInner = (InputStream)f.get(zipIS); + newInner = new ThresholdInputStream(oldInner, null); + f.set(zipIS, newInner); + } catch (Exception ex) { + logger.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); + newInner = null; + } + return newInner; + } + }); } else { // the inner stream is a ZipFileInputStream, i.e. the data wasn't compressed newInner = null; diff --git a/src/ooxml/java/org/apache/poi/util/OOXMLLite.java b/src/ooxml/java/org/apache/poi/util/OOXMLLite.java index c8aa21c214..de19cb0ed9 100644 --- a/src/ooxml/java/org/apache/poi/util/OOXMLLite.java +++ b/src/ooxml/java/org/apache/poi/util/OOXMLLite.java @@ -25,7 +25,9 @@ import java.io.OutputStream; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.net.URL; +import java.security.AccessController; import java.security.CodeSource; +import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Enumeration; @@ -49,7 +51,6 @@ import org.junit.runner.JUnitCore;import org.junit.runner.Result; * @author Yegor Kozlov */ public final class OOXMLLite { - private static Field _classes; /** * Destination directory to copy filtered classes @@ -214,12 +215,19 @@ public final class OOXMLLite { // make the field accessible, we defer this from static initialization to here to // allow JDKs which do not have this field (e.g. IBM JDK) to at least load the class // without failing, see https://issues.apache.org/bugzilla/show_bug.cgi?id=56550 - try { - _classes = ClassLoader.class.getDeclaredField("classes"); - _classes.setAccessible(true); - } catch (Exception e) { - throw new RuntimeException(e); - } + final Field _classes = AccessController.doPrivileged(new PrivilegedAction() { + @SuppressForbidden("TODO: Reflection works until Java 8 on Oracle/Sun JDKs, but breaks afterwards (different classloader types, access checks)") + public Field run() { + try { + Field fld = ClassLoader.class.getDeclaredField("classes"); + fld.setAccessible(true); + return fld; + } catch (Exception e) { + throw new RuntimeException(e); + } + + } + }); ClassLoader appLoader = ClassLoader.getSystemClassLoader(); try { -- 2.39.5