PDFBOX-6185: add creation of temp files; code review and tests by Claude Sonnet

git-svn-id: https://svn.apache.org/repos/asf/pdfbox/trunk@1932922 13f79535-47bb-0310-9956-ffa450edef68

Author: Maruan Sahyoun

io/src/main/java/org/apache/pdfbox/io/IOUtils.java

@@ -45,14 +45,21 @@
 import java.nio.file.attribute.AclEntryPermission;
 import java.nio.file.attribute.AclEntryType;
 import java.nio.file.attribute.AclFileAttributeView;
+import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.nio.file.attribute.UserPrincipal;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 
@@ -75,6 +82,22 @@ public final class IOUtils
     //TODO PDFBox should really use Apache Commons IO.
     private static final Optional<Consumer<ByteBuffer>> UNMAPPER;
 
+    // POSIX file permissions for temporary files and directories (owner read/write/execute only)
+    private static final Set<PosixFilePermission> POSIX_DIR_PERMS =
+        PosixFilePermissions.fromString("rwx------");
+    private static final Set<PosixFilePermission> POSIX_FILE_PERMS =
+        PosixFilePermissions.fromString("rw-------");
+
+    // Derived FileAttribute wrappers for creation-time use
+    private static final FileAttribute<Set<PosixFilePermission>> POSIX_DIR_PERMISSIONS =
+        PosixFilePermissions.asFileAttribute(POSIX_DIR_PERMS);
+    private static final FileAttribute<Set<PosixFilePermission>> POSIX_FILE_PERMISSIONS =
+        PosixFilePermissions.asFileAttribute(POSIX_FILE_PERMS);
+
+    private static final List<Path> TEMP_DIRS_TO_DELETE = Collections.synchronizedList(new ArrayList<>());
+    private static final AtomicBoolean SHUTDOWN_HOOK_REGISTERED = new AtomicBoolean(false);
+
+
     static
     {
         UNMAPPER = Optional.ofNullable(AccessController
@@ -345,80 +368,181 @@ public static StreamCacheCreateFunction createTempFileOnlyStreamCache()
      * other users or processes on the same system. Used e.g. by PDFDebugger.</p>
      * 
      * @return the path to the created temporary directory
-     * @throws IOException 
+     * @throws IOException if an I/O error occurs during directory creation or permission setting
      */
     public static Path createProtectedTempDir() throws IOException
     {
-        // S5443: permissions are immediately restricted to owner-only by
-        // applyOwnerOnlyPermissions(), mitigating the default-permission risk.
-        @SuppressWarnings("java:S5443")
-        Path tempPath = Files.createTempDirectory("pdfbox-");
-        applyOwnerOnlyPermissions(tempPath);
+        Path tempPath;
+        // Set owner-only permissions at file creation time if possible, to minimize the time window where
+        // the file has default permissions.
+        if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix"))
+        {
+            tempPath = Files.createTempDirectory("pdfbox-", POSIX_DIR_PERMISSIONS);
+        }
+        else
+        {
+            // S5443: permissions are immediately restricted to owner-only by
+            // applyOwnerOnlyPermissions(), mitigating the default-permission risk.
+            @SuppressWarnings("java:S5443")
+            Path p = Files.createTempDirectory("pdfbox-");
+            tempPath = p;
+            applyOwnerOnlyPermissions(tempPath, true);
+        }
+
+        registerForDeletion(tempPath);
 
+        return tempPath;
+    }
+
+    private static void registerForDeletion(Path path) {
+        TEMP_DIRS_TO_DELETE.add(path);
         // use shutdown hook instead of deleteOnExit() to ensure deletion
         // of the entire directory in case of not automatically deleted on 
         // JVM exit (e.g. due to open file handles or when the temp directory is not empty)
-        Runtime.getRuntime().addShutdownHook(new Thread(() ->
+        if (SHUTDOWN_HOOK_REGISTERED.compareAndSet(false, true))
         {
-            try (Stream<Path> entries = Files.walk(tempPath))
-            {
-                entries.sorted(Comparator.reverseOrder())
-                    .forEach(p -> p.toFile().delete());
-            }
-            catch (IOException ignored) {}
-        }));
+            Runtime.getRuntime().addShutdownHook(new Thread(() ->
+                TEMP_DIRS_TO_DELETE.forEach(IOUtils::deletePathRecursively)
+            ));
+        }
+    }
 
-        return tempPath;
+    private static void deletePathRecursively(Path path) {
+        try (Stream<Path> entries = Files.walk(path))
+        {
+            entries.sorted(Comparator.reverseOrder())
+                // we are using File.delete() on purpose over Files.deleteIfExists() which would be prefered in general, 
+                // as it's throwing a checked exception. As we are doing that in a shutdown hook there is not much we can
+                // do about it and a logger might no longer be available.
+                .forEach(p -> p.toFile().delete());
+        }
+        catch (IOException ignored) {}
     }
 
-    private static void applyOwnerOnlyPermissions(Path dir) throws IOException
+    /**
+     * Creates a temporary file in the specified directory (or default temporary-file directory 
+     * if null) with owner-only permissions.
+     * 
+     * <p>This method attempts to set owner-only permissions at file creation time when supported,
+     * to minimize the time window during which the file may have default (world-readable) permissions.
+     * On POSIX systems (Linux, macOS), permissions are set during creation. On Windows, permissions
+     * are set after file creation via ACL.</p>
+     * 
+     * <p>Note: This method is designed for storing temporary files that may contain sensitive data
+     * in a temporary directory with restricted permissions, to mitigate the risk of unauthorized 
+     * access by other users or processes on the same system. However, unlike {@link #createProtectedTempDir()},
+     * this method does NOT automatically delete the file on JVM shutdown. The caller is responsible 
+     * for deleting the temporary file when no longer needed. Used e.g. by PDFDebugger.</p>
+     * 
+     * @param dir the directory in which to create the temporary file, or null to use the default 
+     *            temporary-file directory
+     * @param prefix the prefix string to be used in generating the file's name; may be null
+     * @param suffix the suffix string to be used in generating the file's name; may be null
+     * @return the path to the created temporary file with owner-only permissions
+     * @throws IOException if an I/O error occurs during file creation or permission setting
+     * @throws SecurityException if a security manager is installed and denies access
+     * @see #createProtectedTempDir()
+     * @see Files#createTempFile(Path, String, String, FileAttribute[])
+     */
+    public static Path createProtectedTempFile(Path dir, String prefix, String suffix) throws IOException
     {
+        // Set owner-only permissions at file creation time if possible, to minimize the time window where
+        // the file has default permissions.
         if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix"))
         {
-            // Unix/macOS — rwx------
-            Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString("rwx------"));
+            return dir == null 
+                ? Files.createTempFile(prefix, suffix, POSIX_FILE_PERMISSIONS) 
+                : Files.createTempFile(dir, prefix, suffix, POSIX_FILE_PERMISSIONS);
+        }
+        Path tempFile = dir == null 
+            ? Files.createTempFile(prefix, suffix) 
+            : Files.createTempFile(dir, prefix, suffix);
+        applyOwnerOnlyPermissions(tempFile, false);
+        return tempFile;
+    }
+
+    /**
+     * Applies owner-only permissions to a file or directory in a platform-specific manner.
+     * 
+     * <p>This method ensures that the specified file or directory is readable and writable only by its owner,
+     * with no permissions granted to group or others. The implementation differs based on the underlying filesystem:</p>
+     * 
+     * <ul>
+     *   <li><b>POSIX systems (Linux, macOS, Unix):</b> Sets permissions to {@code rwx------} for directories
+     *       or {@code rw-------} for files using POSIX file attributes.</li>
+     *   <li><b>Windows systems:</b> Replaces the entire ACL with a single owner-only ALLOW entry granting full control.
+     *       If ACL is not supported, falls back to using {@link File#setReadable(boolean, boolean)},
+     *       {@link File#setWritable(boolean, boolean)}, and {@link File#setExecutable(boolean, boolean)}.</li>
+     * </ul>
+     * 
+     * <p>If permissions cannot be set successfully on Windows systems, a warning is logged but no exception is thrown.</p>
+     * 
+     * @param path the file or directory to apply owner-only permissions to
+     * @param isDirectory {@code true} if the path is a directory and should have execute permissions;
+     *                    {@code false} if it is a file
+     * @throws IOException if an I/O error occurs while setting POSIX permissions or accessing the file
+     * @throws SecurityException if a security manager is installed and denies access to the file
+     * @see Files#setPosixFilePermissions(Path, Set)
+     * @see Files#getFileAttributeView(Path, Class)
+     */
+    private static void applyOwnerOnlyPermissions(Path path, boolean isDirectory) throws IOException
+    {
+        if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix"))
+        {
+            Set<PosixFilePermission> permissions = isDirectory ? POSIX_DIR_PERMS : POSIX_FILE_PERMS;
+            Files.setPosixFilePermissions(path, permissions);
         }
         else
         {
             // Windows — replace the entire ACL with a single owner-only ALLOW entry
             AclFileAttributeView aclView =
-            Files.getFileAttributeView(dir, AclFileAttributeView.class);
+            Files.getFileAttributeView(path, AclFileAttributeView.class);
 
             if (aclView == null)
             {
-                File tempDir = dir.toFile();
-                boolean isReadable = tempDir.setReadable(true, true);
-                boolean isWritable = tempDir.setWritable(true, true);
-                boolean isExecutable = tempDir.setExecutable(true, true);
-                if (!isReadable || !isWritable || !isExecutable)
+                File pathAsFile = path.toFile();
+                boolean isReadable = pathAsFile.setReadable(true, true);
+                boolean isWritable = pathAsFile.setWritable(true, true);
+                boolean isProtected = isReadable && isWritable;
+                if (isDirectory)
+                {
+                    isProtected &= pathAsFile.setExecutable(true, true);
+                } 
+                if (!isProtected)
                 {
-                    LOG.warn("Unable to set owner-only permissions on temporary directory: {}. " +
-                            "Please ensure that the temporary directory is protected against unauthorized access.",
-                            dir);
+                    LOG.warn("Unable to set owner-only permissions on: {}. " +
+                            "Please ensure that the file or directory is protected against unauthorized access.",
+                            path);
                 }
                 return;
             }
 
             UserPrincipal owner = aclView.getOwner();
 
+            Set<AclEntryPermission> aclPermissions = new HashSet<>(Set.of(
+                AclEntryPermission.READ_DATA,
+                AclEntryPermission.WRITE_DATA,
+                AclEntryPermission.APPEND_DATA,
+                AclEntryPermission.READ_NAMED_ATTRS,
+                AclEntryPermission.WRITE_NAMED_ATTRS,
+                AclEntryPermission.READ_ATTRIBUTES,
+                AclEntryPermission.WRITE_ATTRIBUTES,
+                AclEntryPermission.DELETE,
+                AclEntryPermission.READ_ACL,
+                AclEntryPermission.WRITE_ACL,
+                AclEntryPermission.SYNCHRONIZE
+            ));
+
+            if (isDirectory)
+            {
+                aclPermissions.add(AclEntryPermission.EXECUTE);
+                aclPermissions.add(AclEntryPermission.DELETE_CHILD);
+            }
+
             AclEntry ownerFullControl = AclEntry.newBuilder()
                 .setType(AclEntryType.ALLOW)
                 .setPrincipal(owner)
-                .setPermissions(
-                    AclEntryPermission.READ_DATA,
-                    AclEntryPermission.WRITE_DATA,
-                    AclEntryPermission.APPEND_DATA,
-                    AclEntryPermission.READ_NAMED_ATTRS,
-                    AclEntryPermission.WRITE_NAMED_ATTRS,
-                    AclEntryPermission.EXECUTE,
-                    AclEntryPermission.DELETE_CHILD,
-                    AclEntryPermission.READ_ATTRIBUTES,
-                    AclEntryPermission.WRITE_ATTRIBUTES,
-                    AclEntryPermission.DELETE,
-                    AclEntryPermission.READ_ACL,
-                    AclEntryPermission.WRITE_ACL,
-                    AclEntryPermission.SYNCHRONIZE
-                )
+                .setPermissions(aclPermissions)
                 .build();
 
             // Set so that only the owner has permissions, and remove any inherited ACL entries