Restrict permissions when creating temporary files and directories, or fail if that's not possible.

(Also, check that the provided `fileThreshold` is non-negative.)

- Fixes #2575
- Fixes #4011

RELNOTES=Reimplemented `Files.createTempDir` and `FileBackedOutputStream` to further address [CVE-2020-8908](#4011) and [Guava issue #2575](#2575) (CVE forthcoming).
PiperOrigin-RevId: 535359233

Author: cpovirk

android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java

@@ -16,9 +16,17 @@
 
 package com.google.common.io;
 
+import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
+import static org.junit.Assert.assertThrows;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributes;
 import java.util.Arrays;
 
 /**
@@ -61,10 +69,21 @@ private void testThreshold(
 
     // Write data to go over the threshold
     if (chunk2 > 0) {
+      if (JAVA_IO_TMPDIR.value().equals("/sdcard")) {
+        assertThrows(IOException.class, () -> write(out, data, chunk1, chunk2, singleByte));
+        return;
+      }
       write(out, data, chunk1, chunk2, singleByte);
       file = out.getFile();
       assertEquals(dataSize, file.length());
       assertTrue(file.exists());
+      assertThat(file.getName()).contains("FileBackedOutputStream");
+      if (!isAndroid()) {
+        PosixFileAttributes attributes =
+            java.nio.file.Files.getFileAttributeView(file.toPath(), PosixFileAttributeView.class)
+                .readAttributes();
+        assertThat(attributes.permissions()).containsExactly(OWNER_READ, OWNER_WRITE);
+      }
     }
     out.close();
 
@@ -109,6 +128,10 @@ public void testWriteErrorAfterClose() throws Exception {
     FileBackedOutputStream out = new FileBackedOutputStream(50);
     ByteSource source = out.asByteSource();
 
+    if (JAVA_IO_TMPDIR.value().equals("/sdcard")) {
+      assertThrows(IOException.class, () -> out.write(data));
+      return;
+    }
     out.write(data);
     assertTrue(Arrays.equals(data, source.read()));
 
@@ -140,4 +163,8 @@ public void testReset() throws Exception {
 
     out.close();
   }
+
+  private static boolean isAndroid() {
+    return System.getProperty("java.runtime.name", "").contains("Android");
+  }
 }

android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java

@@ -16,9 +16,17 @@
 
 package com.google.common.io;
 
+import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
 import static com.google.common.truth.Truth.assertThat;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
+import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
+import static org.junit.Assert.assertThrows;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributes;
 import junit.framework.TestCase;
 
 /**
@@ -27,12 +35,32 @@
  * @author Chris Nokleberg
  */
 
+@SuppressWarnings("deprecation") // tests of a deprecated method
 public class FilesCreateTempDirTest extends TestCase {
-  public void testCreateTempDir() {
+  public void testCreateTempDir() throws IOException {
+    if (JAVA_IO_TMPDIR.value().equals("/sdcard")) {
+      assertThrows(IllegalStateException.class, Files::createTempDir);
+      return;
+    }
     File temp = Files.createTempDir();
-    assertTrue(temp.exists());
-    assertTrue(temp.isDirectory());
-    assertThat(temp.listFiles()).isEmpty();
-    assertTrue(temp.delete());
+    try {
+      assertTrue(temp.exists());
+      assertTrue(temp.isDirectory());
+      assertThat(temp.listFiles()).isEmpty();
+
+      if (isAndroid()) {
+        return;
+      }
+      PosixFileAttributes attributes =
+          java.nio.file.Files.getFileAttributeView(temp.toPath(), PosixFileAttributeView.class)
+              .readAttributes();
+      assertThat(attributes.permissions()).containsExactly(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE);
+    } finally {
+      assertTrue(temp.delete());
+    }
+  }
+
+  private static boolean isAndroid() {
+    return System.getProperty("java.runtime.name", "").contains("Android");
   }
 }

android/guava/src/com/google/common/io/FileBackedOutputStream.java

@@ -14,6 +14,7 @@
 
 package com.google.common.io;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
@@ -36,6 +37,14 @@
  * An {@link OutputStream} that starts buffering to a byte array, but switches to file buffering
  * once the data reaches a configurable size.
  *
+ * <p>When this stream creates a temporary file, it restricts the file's permissions to the current
+ * user or, in the case of Android, the current app. If that is not possible (as is the case under
+ * the very old Android Ice Cream Sandwich release), then this stream throws an exception instead of
+ * creating a file that would be more accessible. (This behavior is new in Guava 32.0.0. Previous
+ * versions would create a file that is more accessible, as discussed in <a
+ * href="https://github.com/google/guava/issues/2575">Guava issue 2575</a>. TODO: b/283778848 - Fill
+ * in CVE number once it's available.)
+ *
  * <p>Temporary files created by this stream may live in the local filesystem until either:
  *
  * <ul>
@@ -61,7 +70,6 @@ public final class FileBackedOutputStream extends OutputStream {
   private final int fileThreshold;
   private final boolean resetOnFinalize;
   private final ByteSource source;
-  @CheckForNull private final File parentDirectory;
 
   @GuardedBy("this")
   private OutputStream out;
@@ -97,6 +105,7 @@ synchronized File getFile() {
    * {@link ByteSource} returned by {@link #asByteSource} is finalized.
    *
    * @param fileThreshold the number of bytes before the stream should switch to buffering to a file
+   * @throws IllegalArgumentException if {@code fileThreshold} is negative
    */
   public FileBackedOutputStream(int fileThreshold) {
     this(fileThreshold, false);
@@ -109,16 +118,13 @@ public FileBackedOutputStream(int fileThreshold) {
    * @param fileThreshold the number of bytes before the stream should switch to buffering to a file
    * @param resetOnFinalize if true, the {@link #reset} method will be called when the {@link
    *     ByteSource} returned by {@link #asByteSource} is finalized.
+   * @throws IllegalArgumentException if {@code fileThreshold} is negative
    */
   public FileBackedOutputStream(int fileThreshold, boolean resetOnFinalize) {
-    this(fileThreshold, resetOnFinalize, null);
-  }
-
-  private FileBackedOutputStream(
-      int fileThreshold, boolean resetOnFinalize, @CheckForNull File parentDirectory) {
+    checkArgument(
+        fileThreshold >= 0, "fileThreshold must be non-negative, but was %s", fileThreshold);
     this.fileThreshold = fileThreshold;
     this.resetOnFinalize = resetOnFinalize;
-    this.parentDirectory = parentDirectory;
     memory = new MemoryOutput();
     out = memory;
 
@@ -229,7 +235,7 @@ public synchronized void flush() throws IOException {
   @GuardedBy("this")
   private void update(int len) throws IOException {
     if (memory != null && (memory.getCount() + len > fileThreshold)) {
-      File temp = File.createTempFile("FileBackedOutputStream", null, parentDirectory);
+      File temp = TempFileCreator.INSTANCE.createTempFile("FileBackedOutputStream");
       if (resetOnFinalize) {
         // Finalizers are not guaranteed to be called on system shutdown;
         // this is insurance.

android/guava/src/com/google/common/io/Files.java

@@ -73,9 +73,6 @@
 @ElementTypesAreNonnullByDefault
 public final class Files {
 
-  /** Maximum loop count when creating temp directories. */
-  private static final int TEMP_DIR_ATTEMPTS = 10000;
-
   private Files() {}
 
   /**
@@ -399,17 +396,19 @@ public static boolean equal(File file1, File file2) throws IOException {
    * Atomically creates a new directory somewhere beneath the system's temporary directory (as
    * defined by the {@code java.io.tmpdir} system property), and returns its name.
    *
+   * <p>The temporary directory is created with permissions restricted to the current user or, in
+   * the case of Android, the current app. If that is not possible (as is the case under the very
+   * old Android Ice Cream Sandwich release), then this method throws an exception instead of
+   * creating a directory that would be more accessible. (This behavior is new in Guava 32.0.0.
+   * Previous versions would create a directory that is more accessible, as discussed in <a
+   * href="https://github.com/google/guava/issues/4011">CVE-2020-8908</a>.)
+   *
    * <p>Use this method instead of {@link File#createTempFile(String, String)} when you wish to
    * create a directory, not a regular file. A common pitfall is to call {@code createTempFile},
    * delete the file and create a directory in its place, but this leads a race condition which can
    * be exploited to create security vulnerabilities, especially when executable files are to be
    * written into the directory.
    *
-   * <p>Depending on the environment that this code is run in, the system temporary directory (and
-   * thus the directory this method creates) may be more visible that a program would like - files
-   * written to this directory may be read or overwritten by hostile programs running on the same
-   * machine.
-   *
    * <p>This method assumes that the temporary volume is writable, has free inodes and free blocks,
    * and that it will not be called thousands of times per second.
    *
@@ -418,36 +417,26 @@ public static boolean equal(File file1, File file2) throws IOException {
    *
    * @return the newly-created directory
    * @throws IllegalStateException if the directory could not be created
+   * @throws UnsupportedOperationException if the system does not support creating temporary
+   *     directories securely
    * @deprecated For Android users, see the <a
    *     href="https://developer.android.com/training/data-storage" target="_blank">Data and File
    *     Storage overview</a> to select an appropriate temporary directory (perhaps {@code
-   *     context.getCacheDir()}). For developers on Java 7 or later, use {@link
-   *     java.nio.file.Files#createTempDirectory}, transforming it to a {@link File} using {@link
-   *     java.nio.file.Path#toFile() toFile()} if needed.
+   *     context.getCacheDir()}), and create your own directory under that. (For example, you might
+   *     use {@code new File(context.getCacheDir(), "directoryname").mkdir()}, or, if you need an
+   *     arbitrary number of temporary directories, you might have to generate multiple directory
+   *     names in a loop until {@code mkdir()} returns {@code true}.) For developers on Java 7 or
+   *     later, use {@link java.nio.file.Files#createTempDirectory}, transforming it to a {@link
+   *     File} using {@link java.nio.file.Path#toFile() toFile()} if needed. To restrict permissions
+   *     as this method does, pass {@code
+   *     PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"))} to your
+   *     call to {@code createTempDirectory}.
    */
   @Beta
   @Deprecated
   @J2ObjCIncompatible
   public static File createTempDir() {
-    File baseDir = new File(System.getProperty("java.io.tmpdir"));
-    @SuppressWarnings("GoodTime") // reading system time without TimeSource
-    String baseName = System.currentTimeMillis() + "-";
-
-    for (int counter = 0; counter < TEMP_DIR_ATTEMPTS; counter++) {
-      File tempDir = new File(baseDir, baseName + counter);
-      if (tempDir.mkdir()) {
-        return tempDir;
-      }
-    }
-    throw new IllegalStateException(
-        "Failed to create directory within "
-            + TEMP_DIR_ATTEMPTS
-            + " attempts (tried "
-            + baseName
-            + "0 to "
-            + baseName
-            + (TEMP_DIR_ATTEMPTS - 1)
-            + ')');
+    return TempFileCreator.INSTANCE.createTempDir();
   }
 
   /**

android/guava/src/com/google/common/io/IgnoreJRERequirement.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2019 The Guava Authors
+ *
+ * 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.google.common.io;
+
+import static java.lang.annotation.ElementType.CONSTRUCTOR;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+
+import java.lang.annotation.Target;
+
+/**
+ * Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
+ *
+ * <p>Each package's copy of this annotation needs to be listed in our {@code pom.xml}.
+ */
+@Target({METHOD, CONSTRUCTOR, TYPE})
+@ElementTypesAreNonnullByDefault
+@interface IgnoreJRERequirement {}