From c8975a9afa6d347a91525d9f24c94d66fd979004 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 --- .../common/io/FileBackedOutputStreamTest.java | 27 ++- .../common/io/FilesCreateTempDirTest.java | 66 +++++++ .../common/io/FileBackedOutputStream.java | 25 ++- .../guava/src/com/google/common/io/Files.java | 49 ++--- .../common/io/IgnoreJRERequirement.java | 30 +++ .../com/google/common/io/TempFileCreator.java | 173 ++++++++++++++++++ android/pom.xml | 3 +- .../common/io/FileBackedOutputStreamTest.java | 28 ++- .../common/io/FilesCreateTempDirTest.java | 66 +++++++ .../common/io/FileBackedOutputStream.java | 24 ++- guava/src/com/google/common/io/Files.java | 49 ++--- .../common/io/IgnoreJRERequirement.java | 29 +++ .../com/google/common/io/TempFileCreator.java | 173 ++++++++++++++++++ pom.xml | 3 +- 14 files changed, 663 insertions(+), 82 deletions(-) create mode 100644 android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java create mode 100644 android/guava/src/com/google/common/io/IgnoreJRERequirement.java create mode 100644 android/guava/src/com/google/common/io/TempFileCreator.java create mode 100644 guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java 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/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java index 6841a41a9742b..5ffa9a42412bd 100644 --- a/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java +++ b/android/guava-tests/test/com/google/common/io/FileBackedOutputStreamTest.java @@ -16,11 +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 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 +67,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(); @@ -133,6 +150,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())); @@ -164,4 +185,8 @@ public void testReset() throws Exception { out.close(); } + + private static boolean isAndroid() { + return System.getProperty("java.runtime.name", "").contains("Android"); + } } diff --git a/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java new file mode 100644 index 0000000000000..62098ef0288ae --- /dev/null +++ b/android/guava-tests/test/com/google/common/io/FilesCreateTempDirTest.java @@ -0,0 +1,66 @@ +/* + * Copyright (C) 2007 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 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}. + * + * @author Chris Nokleberg + */ + +@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(); + 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/android/guava/src/com/google/common/io/FileBackedOutputStream.java b/android/guava/src/com/google/common/io/FileBackedOutputStream.java index d8c4189546fe5..595eade2bb46d 100644 --- a/android/guava/src/com/google/common/io/FileBackedOutputStream.java +++ b/android/guava/src/com/google/common/io/FileBackedOutputStream.java @@ -14,6 +14,8 @@ 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; import com.google.common.annotations.GwtIncompatible; @@ -33,6 +35,14 @@ * 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: * *