From feb83a1c8fd2e7670b244d5afd23cba5aca43284 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 25 May 2023 13:18:00 -0700 Subject: [PATCH] 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 https://github.com/google/guava/issues/2575 - Fixes https://github.com/google/guava/issues/4011 RELNOTES=Reimplemented `Files.createTempDir` and `FileBackedOutputStream` to further address [CVE-2020-8908](https://github.com/google/guava/issues/4011) and [Guava issue #2575](https://github.com/google/guava/issues/2575) (CVE forthcoming). PiperOrigin-RevId: 535359233 Refer: https://github.com/google/guava/commit/feb83a1c8fd2e7670b244d5afd23cba5aca43284 --- .../common/io/FileBackedOutputStreamTest.java | 27 +++ .../common/io/FilesCreateTempDirTest.java | 41 +++- .../common/io/FileBackedOutputStream.java | 22 ++- guava/src/com/google/common/io/Files.java | 51 ++--- .../common/io/IgnoreJRERequirement.java | 30 +++ .../com/google/common/io/TempFileCreator.java | 176 ++++++++++++++++++ 6 files changed, 302 insertions(+), 45 deletions(-) create mode 100644 guava/src/com/google/common/io/IgnoreJRERequirement.java create mode 100644 guava/src/com/google/common/io/TempFileCreator.java diff --git a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java index 6841a41..2dae0ed 100644 --- a/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java +++ b/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java @@ -17,10 +17,18 @@ 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 com.google.common.testing.GcFinalization; 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 @@ public class FileBackedOutputStreamTest extends IoTestCase { // 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(); @@ -133,6 +152,10 @@ public class FileBackedOutputStreamTest extends IoTestCase { 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())); @@ -164,4 +187,8 @@ public class FileBackedOutputStreamTest extends IoTestCase { out.close(); } + + private static boolean isAndroid() { + return System.getProperty("java.runtime.name", "").contains("Android"); + } } diff --git a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java index 557689e..62098ef 100644 --- a/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java +++ b/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -16,9 +16,18 @@ 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; /** * Unit test for {@link Files#createTempDir}. @@ -26,12 +35,32 @@ import java.io.File; * @author Chris Nokleberg */ -public class FilesCreateTempDirTest extends IoTestCase { - public void testCreateTempDir() { +@SuppressWarnings("deprecation") // tests of a deprecated method +public class FilesCreateTempDirTest extends TestCase { + 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"); } } diff --git a/guava/src/com/google/common/io/FileBackedOutputStream.java b/guava/src/com/google/common/io/FileBackedOutputStream.java index 9912e2f..ef553a0 100644 --- a/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/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; @@ -34,6 +35,14 @@ import javax.annotation.CheckForNull; * An {@link OutputStream} that starts buffering to a byte array, but switches to file buffering * once the data reaches a configurable size. * + *

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 Guava issue 2575. TODO: b/283778848 - Fill + * in CVE number once it's available.) + * *

Temporary files created by this stream may live in the local filesystem until either: * *