Fix path traversal in RenderSecurityManager

RenderSecurityManager would use not normalize paths allowing for
read/write operations to avoid the simple string checks.

Test: RenderSecurityManagerTest
Fixes: 236865896
Change-Id: I658aecfdea718f9297f5ac26219834651dd9cbc1

5 files changed, 281 insertions, 106 deletions

diff --git a/android-kotlin/idea-android/tests/org/jetbrains/kotlin/android/KotlinAndroidTestCase.java b/android-kotlin/idea-android/tests/org/jetbrains/kotlin/android/KotlinAndroidTestCase.java
index 31670a55378..f256bd7d9db 100644
--- a/android-kotlin/idea-android/tests/org/jetbrains/kotlin/android/KotlinAndroidTestCase.java
+++ b/android-kotlin/idea-android/tests/org/jetbrains/kotlin/android/KotlinAndroidTestCase.java
@@ -109,11 +109,6 @@ public abstract class KotlinAndroidTestCase extends UsefulTestCase {
       }
     }
 
-    if (RenderSecurityManager.RESTRICT_READS) {
-      // Unit test class loader includes disk directories which security manager does not allow access to
-      RenderSecurityManager.sEnabled = false;
-    }
-
     mySettings = CodeStyleSettingsManager.getSettings(getProject()).clone();
     // Note: we apply the Android Studio code style so that tests running as the Android plugin in IDEA behave the same.
     AndroidTestCase.applyAndroidCodeStyleSettings(mySettings);
@@ -147,9 +142,6 @@ public abstract class KotlinAndroidTestCase extends UsefulTestCase {
       mySettings = null;
 
       getAndroidCodeStyleSettings().USE_CUSTOM_SETTINGS = myUseCustomSettings;
-      if (RenderSecurityManager.RESTRICT_READS) {
-        RenderSecurityManager.sEnabled = true;
-      }
     }
     finally {
       try {
diff --git a/android-test-framework/testSrc/org/jetbrains/android/AndroidTestCase.java b/android-test-framework/testSrc/org/jetbrains/android/AndroidTestCase.java
index 4c00de584e0..a339d12bdec 100644
--- a/android-test-framework/testSrc/org/jetbrains/android/AndroidTestCase.java
+++ b/android-test-framework/testSrc/org/jetbrains/android/AndroidTestCase.java
@@ -155,11 +155,6 @@ public abstract class AndroidTestCase extends AndroidTestBase {
       deleteManifest();
     }
 
-    if (RenderSecurityManager.RESTRICT_READS) {
-      // Unit test class loader includes disk directories which security manager does not allow access to
-      RenderSecurityManager.sEnabled = false;
-    }
-
     ArrayList<String> allowedRoots = new ArrayList<>();
     collectAllowedRoots(allowedRoots);
     registerAllowedRoots(allowedRoots, getTestRootDisposable());
@@ -216,9 +211,6 @@ public abstract class AndroidTestCase extends AndroidTestBase {
       mySettings = null;
 
       getAndroidCodeStyleSettings().USE_CUSTOM_SETTINGS = myUseCustomSettings;
-      if (RenderSecurityManager.RESTRICT_READS) {
-        RenderSecurityManager.sEnabled = true;
-      }
     }
     finally {
       try {
diff --git a/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java b/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
index c52665d161d..0600c93d820 100644
--- a/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
+++ b/android/src/com/android/tools/idea/rendering/RenderSecurityManager.java
@@ -24,8 +24,10 @@ import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FilePermission;
 import java.io.IOException;
-import java.lang.reflect.Member;
 import java.net.InetAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.security.Permission;
 import java.util.Arrays;
 import java.util.PropertyPermission;
@@ -50,11 +52,6 @@ public class RenderSecurityManager extends SecurityManager {
   public static final String ENABLED_PROPERTY = "android.render.sandbox";
 
   /**
-   * Whether we should restrict reading to certain paths
-   */
-  public static final boolean RESTRICT_READS = false;
-
-  /**
    * Whether the security manager is enabled for this session (it might still
    * be inactive, either because it's active for a different thread, or because
    * it has been disabled via {@link #setActive(boolean, Object)} (which sets the
@@ -106,6 +103,18 @@ public class RenderSecurityManager extends SecurityManager {
   private SecurityManager myPreviousSecurityManager;
   private ILogger mLogger;
 
+  private boolean isRestrictReads;
+
+  @NotNull
+  private static String normalizeDirectoryPath(@NotNull Path originalPath) {
+    return originalPath.normalize() + originalPath.getFileSystem().getSeparator();
+  }
+
+  @NotNull
+  private static String normalizeDirectoryPath(@NotNull String stringPath) {
+    return normalizeDirectoryPath(Paths.get(stringPath));
+  }
+
   /**
    * Returns the current render security manager, if any. This will only return
    * non-null if there is an active {@linkplain RenderSecurityManager} as the
@@ -140,17 +149,23 @@ public class RenderSecurityManager extends SecurityManager {
    *                    this is used to allow specific path prefixes for layoutlib resource
    *                    lookup
    * @param projectPath a path to the project directory, used for similar purposes
+   * @param restrictReads when true, reads will be restricted to only a set of directories including temp directory and the given sdkPath
+   *                      and projectPath directories.
    */
-  public RenderSecurityManager(@Nullable String sdkPath, @Nullable String projectPath) {
+  public RenderSecurityManager(
+    @Nullable String sdkPath,
+    @Nullable String projectPath,
+    boolean restrictReads) {
     mSdkPath = sdkPath;
     mProjectPath = projectPath;
     mTempDir = System.getProperty("java.io.tmpdir");
     mNormalizedTempDir = new File(mTempDir).getPath(); // will call fs.normalize() on the path
-    mIndexRootPath = PathManager.getIndexRoot().toString();
-    mLogRootPath = PathManager.getLogPath();
-    mCachePath = PathManager.getSystemPath() + "/caches/";
+    mIndexRootPath = normalizeDirectoryPath(PathManager.getIndexRoot());
+    mLogRootPath = normalizeDirectoryPath(PathManager.getLogPath());
+    mCachePath = normalizeDirectoryPath(Paths.get(PathManager.getSystemPath(), "caches"));
     //noinspection AssignmentToStaticFieldFromInstanceMethod
     sLastFailedPath = null;
+    isRestrictReads = restrictReads;
   }
 
   /**
@@ -369,7 +384,7 @@ public class RenderSecurityManager extends SecurityManager {
   @SuppressWarnings({"PointlessBooleanExpression", "ConstantConditions"})
   @Override
   public void checkRead(String file) {
-    if (RESTRICT_READS && isRelevant() && !isReadingAllowed(file)) {
+    if (isRestrictReads && isRelevant() && !isReadingAllowed(file)) {
       throw RenderSecurityException.create("Read", file);
     }
   }
@@ -377,13 +392,20 @@ public class RenderSecurityManager extends SecurityManager {
   @SuppressWarnings({"PointlessBooleanExpression", "ConstantConditions"})
   @Override
   public void checkRead(String file, Object context) {
-    if (RESTRICT_READS && isRelevant() && !isReadingAllowed(file)) {
+    if (isRestrictReads && isRelevant() && !isReadingAllowed(file)) {
       throw RenderSecurityException.create("Read", file);
     }
   }
 
   private boolean isReadingAllowed(String path) {
-    if (RESTRICT_READS) {
+    if (isRestrictReads) {
+      try {
+        path = canonicalize(path);
+      }
+      catch (IOException e) {
+        return false;
+      }
+
       // Allow reading files in the SDK install (fonts etc)
       if (mSdkPath != null && path.startsWith(mSdkPath)) {
         return true;
@@ -430,8 +452,21 @@ public class RenderSecurityManager extends SecurityManager {
     }
   }
 
+  private static String canonicalize(@NotNull String path) throws IOException {
+    return Paths.get(path).normalize().toFile().getCanonicalPath();
+  }
+
   @SuppressWarnings("RedundantIfStatement")
   private boolean isWritingAllowed(String path) {
+    try {
+      path = canonicalize(path);
+      // We do not allow writing to links of paths that do not exist. If the path had existed, it would have
+      // been resolved by the canonicalize call.
+      if (Files.isSymbolicLink(Paths.get(path))) return false;
+    }
+    catch (IOException e) {
+      return false;
+    }
     return isTempDirPath(path) ||
            // When loading classes, IntelliJ might sometimes drop a corruption marker
            path.startsWith(mIndexRootPath) ||
@@ -453,10 +488,10 @@ public class RenderSecurityManager extends SecurityManager {
     // Work around weird temp directories
     try {
       if (mCanonicalTempDir == null) {
-        mCanonicalTempDir = new File(mNormalizedTempDir).getCanonicalPath();
+        mCanonicalTempDir = canonicalize(mNormalizedTempDir);
       }
 
-      if (path.startsWith(mCanonicalTempDir) || new File(path).getCanonicalPath().startsWith(mCanonicalTempDir)) {
+      if (path.startsWith(mCanonicalTempDir) || canonicalize(path).startsWith(mCanonicalTempDir)) {
         return true;
       }
     }
@@ -668,10 +703,14 @@ public class RenderSecurityManager extends SecurityManager {
         throw RenderSecurityException.create("Window", null);
       }
     }
-    else if (isRelevant()) {
+    else if ("symbolic".equals(name)) {
+      if (isRelevant()) {
+        throw RenderSecurityException.create("SymbolicLinks", null);
+      }
+    } else if (isRelevant()) {
       String actions = permission.getActions();
       //noinspection PointlessBooleanExpression,ConstantConditions
-      if (RESTRICT_READS && "read".equals(actions)) {
+      if (isRestrictReads && "read".equals(actions)) {
         if (!isReadingAllowed(name)) {
           throw RenderSecurityException.create("Read", name);
         }
diff --git a/android/src/com/android/tools/idea/rendering/RenderSecurityManagerFactory.java b/android/src/com/android/tools/idea/rendering/RenderSecurityManagerFactory.java
index 1bac91cd963..62569cb0846 100644
--- a/android/src/com/android/tools/idea/rendering/RenderSecurityManagerFactory.java
+++ b/android/src/com/android/tools/idea/rendering/RenderSecurityManagerFactory.java
@@ -19,6 +19,8 @@ import com.android.tools.idea.log.LogWrapper;
 import com.intellij.openapi.application.PathManager;
 import com.intellij.openapi.module.Module;
 import org.jetbrains.android.sdk.AndroidPlatform;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Factory of {@link RenderSecurityManager}.
@@ -28,18 +30,13 @@ public class RenderSecurityManagerFactory {
    * Returns a {@link RenderSecurityManager} for the current module. The {@link RenderSecurityManager} will be
    * setup with the SDK path and project path of the module.
    */
-  public static RenderSecurityManager create(Module module, AndroidPlatform platform) {
-    String projectPath = null;
-    String sdkPath = null;
-    if (RenderSecurityManager.RESTRICT_READS) {
-      projectPath = module.getProject().getBasePath();
-      if (platform != null) {
-        sdkPath = platform.getSdkData().getLocation().toString();
-      }
-    }
+  @NotNull
+  public static RenderSecurityManager create(@NotNull Module module, @Nullable AndroidPlatform platform) {
+    String projectPath = module.getProject().getBasePath();
+    String sdkPath = platform != null ? platform.getSdkData().getLocation().toString() : null;
 
     @SuppressWarnings("ConstantConditions")
-    RenderSecurityManager securityManager = new RenderSecurityManager(sdkPath, projectPath);
+    RenderSecurityManager securityManager = new RenderSecurityManager(sdkPath, projectPath, false);
     securityManager.setLogger(new LogWrapper(RenderLogger.LOG).alwaysLogAsDebug(true).allowVerbose(false));
     securityManager.setAppTempDir(PathManager.getTempPath());
 
diff --git a/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java b/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
index 7bc0b824b09..6b5041052c9 100644
--- a/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
+++ b/android/testSrc/com/android/tools/idea/rendering/RenderSecurityManagerTest.java
@@ -22,6 +22,9 @@ import com.google.common.io.Files;
 import java.io.IOException;
 import com.intellij.openapi.application.PathManager;
 import com.intellij.openapi.diagnostic.Logger;
+import java.nio.charset.Charset;
+import java.nio.file.Path;
+import java.util.UUID;
 import org.jetbrains.android.AndroidTestBase;
 import org.junit.Rule;
 import org.junit.Test;
@@ -55,7 +58,7 @@ public class RenderSecurityManagerTest {
   @Test
   public void testExec() throws IOException {
     assertNull(RenderSecurityManager.getCurrent());
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     RecordingLogger logger = new RecordingLogger();
     manager.setLogger(logger);
     try {
@@ -71,9 +74,7 @@ public class RenderSecurityManagerTest {
       fail("Should have thrown security exception");
     }
     catch (SecurityException exception) {
-      assertEquals(RenderSecurityManager.RESTRICT_READS
-                          ? "Read access not allowed during rendering (/bin/ls)"
-                          : "Exec access not allowed during rendering (/bin/ls)", exception.toString());
+      assertEquals("Exec access not allowed during rendering (/bin/ls)", exception.toString());
       // pass
     }
     finally {
@@ -86,7 +87,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testSetSecurityManager() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
       System.setSecurityManager(null);
@@ -102,25 +103,112 @@ public class RenderSecurityManagerTest {
   }
 
   @Test
-  public void testReadWrite() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
-    try {
-      manager.setActive(true, myCredential);
-      manager.checkPermission(new FilePermission("/foo", "read,write"));
-      fail("Should have thrown security exception");
+  public void testRead() {
+    RenderSecurityManager manager = new RenderSecurityManager(
+      "/Users/userHome/Sdk",
+      "/Users/userHome/Projects/project1",
+      true);
+
+    // Not allowed paths
+    String[] notAllowedPaths = new String[] {
+      "/foo",
+      "/Users/userHome/Sdk/../foo",
+      "/Users/userHome",
+      "/Users/userHome/Projects/project1/../../test",
+    };
+    for (String path: notAllowedPaths) {
+      try {
+        manager.setActive(true, myCredential);
+        manager.checkPermission(new FilePermission(path, "read"));
+        fail(String.format("Should have thrown security exception (%s)", path));
+      }
+      catch (SecurityException exception) {
+        assertEquals(
+          String.format("Read access not allowed during rendering (%s)", path), exception.toString());
+        // pass
+      }
+      finally {
+        manager.dispose(myCredential);
+      }
     }
-    catch (SecurityException exception) {
-      assertEquals("Write access not allowed during rendering (/foo)", exception.toString());
-      // pass
+
+    // allowed paths
+    String[] allowedReadPaths = new String[] {
+      "/Users/userHome/Sdk/foo",
+      "/Users/userHome/Sdk/foo.jar",
+      "/Users/userHome/Sdk/foo/test",
+      "/Users/userHome/Sdk/foo/../foo.jar",
+      "/Users/userHome/Sdk/foo/../../Sdk/test/foo.jar",
+      "/Users/userHome/Projects/project1/path/test.kt",
+      "/Users/userHome/Projects/project1/test.kt",
+      "/Users/userHome/Projects/project1/test.kt",
+      "/Users/userHome/Projects/project1/../project1/test.kt",
+    };
+    for (String path: allowedReadPaths) {
+      try {
+        manager.setActive(true, myCredential);
+        manager.checkPermission(new FilePermission(path, "read"));
+      }
+      finally {
+        manager.dispose(myCredential);
+      }
     }
-    finally {
-      manager.dispose(myCredential);
+  }
+
+  @Test
+  public void testWrite() {
+    RenderSecurityManager manager = new RenderSecurityManager(
+      "/Users/userHome/Sdk",
+      "/Users/userHome/Projects/project1",
+      false);
+
+    String cachePath = PathManager.getSystemPath() + "/caches/";
+    String indexPath = PathManager.getIndexRoot() + "/";
+
+    // Not allowed paths
+    String[] notAllowedPaths = new String[] {
+      "foo",
+      cachePath,
+      indexPath,
+      cachePath + "../foo",
+      cachePath + "../../test.jar",
+      indexPath + "../foo",
+    };
+    for (String path: notAllowedPaths) {
+      try {
+        manager.setActive(true, myCredential);
+        manager.checkPermission(new FilePermission(path, "write"));
+        fail(String.format("Should have thrown security exception (%s)", path));
+      }
+      catch (SecurityException exception) {
+        assertEquals(
+          String.format("Write access not allowed during rendering (%s)", path), exception.toString());
+        // pass
+      }
+      finally {
+        manager.dispose(myCredential);
+      }
+    }
+
+    // allowed paths
+    String[] allowedWritePaths = new String[] {
+      cachePath + "/test/../test.jar",
+      cachePath + "/foo.jar",
+    };
+    for (String path: allowedWritePaths) {
+      try {
+        manager.setActive(true, myCredential);
+        manager.checkPermission(new FilePermission(path, "write"));
+      }
+      finally {
+        manager.dispose(myCredential);
+      }
     }
   }
 
   @Test
   public void testExecute() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
       manager.checkPermission(new FilePermission("/foo", "execute"));
@@ -137,7 +225,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testDelete() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
       manager.checkPermission(new FilePermission("/foo", "delete"));
@@ -154,7 +242,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testLoadLibrary() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -175,7 +263,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testAllowedLoadLibrary() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -192,33 +280,17 @@ public class RenderSecurityManagerTest {
   @SuppressWarnings("CheckReturnValue")
   @Test
   public void testInvalidRead() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
-      if (RenderSecurityManager.RESTRICT_READS) {
-        try {
-          File file = new File(System.getProperty("user.home"));
-          //noinspection ResultOfMethodCallIgnored
-          file.lastModified();
-
-          fail("Should have thrown security exception");
-        }
-        catch (SecurityException exception) {
-          assertEquals("Read access not allowed during rendering (" +
-                              System.getProperty("user.home") + ")", exception.toString());
-          // pass
-        }
+      try {
+        File file = new File(System.getProperty("user.home"));
+        //noinspection ResultOfMethodCallIgnored
+        file.lastModified();
       }
-      else {
-        try {
-          File file = new File(System.getProperty("user.home"));
-          //noinspection ResultOfMethodCallIgnored
-          file.lastModified();
-        }
-        catch (SecurityException exception) {
-          fail("Reading should be allowed");
-        }
+      catch (SecurityException exception) {
+        fail("Reading should be allowed");
       }
     }
     finally {
@@ -228,7 +300,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testInvalidPropertyWrite() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -250,7 +322,7 @@ public class RenderSecurityManagerTest {
   @SuppressWarnings("CheckReturnValue")
   @Test
   public void testReadOk() throws IOException {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -273,7 +345,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testProperties() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -293,7 +365,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testExit() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -326,7 +398,7 @@ public class RenderSecurityManagerTest {
         }
       }
     };
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -358,7 +430,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testActive() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -427,7 +499,7 @@ public class RenderSecurityManagerTest {
           barrier1.await();
           assertNull(RenderSecurityManager.getCurrent());
 
-          RenderSecurityManager manager = new RenderSecurityManager(null, null);
+          RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
           manager.setActive(true, myCredential);
 
           barrier2.await();
@@ -555,7 +627,7 @@ public class RenderSecurityManagerTest {
   public void testDisabled() {
     assertNull(RenderSecurityManager.getCurrent());
 
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     RenderSecurityManager.sEnabled = false;
     try {
       assertNull(RenderSecurityManager.getCurrent());
@@ -614,7 +686,7 @@ public class RenderSecurityManagerTest {
     });
     thread.start();
 
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     RecordingLogger logger = new RecordingLogger();
     manager.setLogger(logger);
     try {
@@ -646,7 +718,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testEnterExitSafeRegion() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     Object credential = new Object();
     try {
       manager.setActive(true, credential);
@@ -721,7 +793,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testRunSafeRegion() throws Exception {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     Object credential = new Object();
     try {
       manager.setActive(true, credential);
@@ -767,7 +839,7 @@ public class RenderSecurityManagerTest {
   @SuppressWarnings("UnstableApiUsage")
   @Test
   public void testImageIo() throws IOException, InterruptedException {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -811,7 +883,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testTempDir() throws IOException {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -832,7 +904,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testAppTempDir() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setAppTempDir("/random/path/");
       manager.setActive(true, myCredential);
@@ -845,7 +917,7 @@ public class RenderSecurityManagerTest {
 
   @Test
   public void testSetTimeZone() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -876,15 +948,13 @@ public class RenderSecurityManagerTest {
    */
   @Test
   public void testLogDir() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
       String logPath = PathManager.getLogPath();
       assertNotNull(logPath);
 
-      manager.checkPermission(new FilePermission(logPath, "read,write"));
-      manager.checkPermission(new FilePermission(logPath + separator, "read,write"));
       manager.checkPermission(new FilePermission(logPath + separator + "fake.log", "read,write"));
 
     }
@@ -894,7 +964,7 @@ public class RenderSecurityManagerTest {
   }
   @Test
   public void testLogException() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);
 
@@ -920,8 +990,93 @@ public class RenderSecurityManagerTest {
   }
 
   @Test
+  public void testNoLinkCreationAllowed() throws IOException {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
+    Path testTemp = java.nio.file.Files.createTempDirectory("linkTest");
+    Path attackLink = testTemp.resolve("attack");
+    File victimFile = new File(PathManager.getConfigPath() + "/victim-" + UUID.randomUUID().toString());
+    victimFile.deleteOnExit();
+    try {
+      manager.setActive(true, myCredential);
+      java.nio.file.Files.createSymbolicLink(attackLink, victimFile.toPath());
+      fail("Should have thrown security exception");
+    }
+    catch (SecurityException exception) {
+      assertThat(exception.toString()).startsWith("SymbolicLinks access not allowed during rendering");
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+
+  @Test
+  public void testCheckWriteToNonExistingLink() throws IOException {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
+
+    Path testTemp = java.nio.file.Files.createTempDirectory("linkTest");
+    Path attackLink = testTemp.resolve("attack");
+    File victimFile = new File(PathManager.getConfigPath() + "/victim-" + UUID.randomUUID());
+    victimFile.deleteOnExit();
+    java.nio.file.Files.createSymbolicLink(attackLink, victimFile.toPath());
+
+    try {
+      manager.setActive(true, myCredential);
+      manager.checkPermission(new FilePermission(attackLink.toString(), "read,write"));
+      fail("Should have thrown security exception");
+    }
+    catch (SecurityException exception) {
+      assertThat(exception.toString()).startsWith("Write access not allowed during rendering");
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+
+  @Test
+  public void testCheckWriteToExistingLink() throws IOException {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
+
+    Path testTemp = java.nio.file.Files.createTempDirectory("linkTest");
+    Path attackLink = testTemp.resolve("attack");
+    File victimFile = new File(PathManager.getConfigPath() + "/victim-" + UUID.randomUUID());
+    victimFile.deleteOnExit();
+    java.nio.file.Files.createSymbolicLink(attackLink, victimFile.toPath());
+    java.nio.file.Files.writeString(victimFile.toPath(), "existing-file", Charset.defaultCharset());
+    try {
+      manager.setActive(true, myCredential);
+      manager.checkPermission(new FilePermission(attackLink.toString(), "read,write"));
+      fail("Should have thrown security exception");
+    }
+    catch (SecurityException exception) {
+      assertThat(exception.toString()).startsWith("Write access not allowed during rendering");
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+
+  /**
+   * Regression test for b/236865896.
+   */
+  @Test
+  public void testPathTraversal() throws IOException {
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
+    try {
+      manager.setActive(true, myCredential);
+      manager.checkPermission(new FilePermission("/tmp/../dev/null", "read,write"));
+      fail("Should have thrown security exception");
+    }
+    catch (SecurityException exception) {
+      assertThat(exception.toString()).startsWith("Write access not allowed during rendering");
+    }
+    finally {
+      manager.dispose(myCredential);
+    }
+  }
+
+  @Test
   public void testSystemPropertiesAccess() {
-    RenderSecurityManager manager = new RenderSecurityManager(null, null);
+    RenderSecurityManager manager = new RenderSecurityManager(null, null, false);
     try {
       manager.setActive(true, myCredential);