Re: [PATCH 2/2] Ignore failure of setting mode on a temporary file on OS/2

[Zack Weinberg]

On Sun, Oct 15, 2023, at 3:43 AM, KO Myung-Hun wrote:
> Zack Weinberg wrote:
>> On Sat, Oct 14, 2023, at 9:19 AM, KO Myung-Hun wrote:
>>> * bin/autom4te.in (handle_output): Ignore setting mode failure on OS/2.
>>
>> Not OK, for two reasons:
...
> How about this ?
> 1. create and close a temporary file
> 2. chmod() on it
> 3. re-open it with O_TRUNC ?

Multi-user security is probably not a concern for modern-day use of
OS/2.  Also, the temporary files created by the code we’re talking
about are not created in a system-wide scratch directory.  So we
probably *could* do it this way, but I don’t like it, because it’s
not safe in the general case.

The trouble is, on a multi-user system, any time you do any operation
by name on a file whose full pathname includes a world-writable
directory (such as the system-wide scratch directories), even if that
directory is “sticky” (chmod +t), you have to be exquisitely careful,
or a malicious concurrent process might be able to trick you into
overwriting some file elsewhere on the filesystem.  For example, your
steps 2 and 3, if executed as root on a file expected to exist in
/tmp, would give a malicious concurrent process a chance to clobber
the access control bits and/or the contents of *any file*, by moving
the temporary file out of the way, and replacing it with a symlink,
in between steps 1 and 2.  That’s a narrow race window to hit, but
it has been done successfully in the past.

I don’t want to have code in Autoconf that is only safe because of
non-obvious details of the context it’s used in.  People might
reuse the code in a different context where it’s *not* safe, without
realizing the danger.  So I suggest we use the appended patch
instead of your patch.  I’ve tested this on Unix systems with both
Perl 5.6 and Perl 5.38.  Could you please test it on your OS/2 system
as well?

zw

---
diff --git a/bin/autom4te.in b/bin/autom4te.in
index 71d7e6a6..41da77b0 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -222,6 +222,52 @@ Written by Akim Demaille.
 ## Routines.  ##
 ## ---------- ##
 
+# tempfile_with_mode ($dir, $mode)
+# --------------------------------
+# Create a temporary file in $dir with access control bits $mode.
+# Returns a list ($fh, $fname) where $fh is a filehandle open for
+# writing to the file, and $fname is the name of the file.
+sub tempfile_with_mode ($$)
+{
+  my ($dir, $mode) = @_;
+
+  require File::Temp;
+  my $template = "actmp." . File::Temp::TEMPXXX;
+
+  # The PERMS argument was added to File::Temp::tempfile in version
+  # 0.2310 of the File::Temp module; it will be silently ignored if
+  # passed to an older version of the function.  This is the simplest
+  # way to do a non-fatal version check without features of Perl 5.10.
+  local $@;
+  if (eval { File::Temp->VERSION("0.2310"); 1 })
+    {
+      # Can use PERMS argument to tempfile().
+      return File::Temp::tempfile ($template, DIR => $dir, PERMS => $mode,
+                                   UNLINK => 0);
+    }
+  else
+    {
+      # PERMS is not available.
+      # This is functionally equivalent to what it would do.
+      require Fcntl;
+      my $openflags = Fcntl::O_RDWR | Fcntl::O_CREAT | Fcntl::O_EXCL;
+
+      require File::Spec;
+      $template = File::Spec->catfile($dir, $template);
+
+      # 50 = $MAX_GUESS in File::Temp (not an exported constant).
+      for (my $i = 0; $i < 50; $i++)
+        {
+          my $filename = File::Temp::mktemp($template);
+          my $fh;
+          my $success = sysopen ($fh, $filename, $openflags, $mode);
+          return ($fh, $filename) if $success;
+          fatal "Could not create temp file $filename: $!"
+            unless $!{EEXIST};
+        }
+      fatal "Could not create any temp file from $template: $!";
+    }
+}
 
 # $OPTION
 # files_to_options (@FILE)
@@ -563,15 +609,7 @@ sub handle_output ($$)
   else
     {
       my (undef, $outdir, undef) = fileparse ($output);
-
-      use File::Temp qw (tempfile);
-      ($out, $scratchfile) = tempfile (UNLINK => 0, DIR => $outdir);
-      fatal "cannot create a file in $outdir: $!"
-        unless $out;
-
-      # File::Temp doesn't give us access to 3-arg open(2), unfortunately.
-      chmod (oct ($mode) & ~(umask), $scratchfile)
-        or fatal "setting mode of " . $scratchfile . ": $!";
+      ($out, $scratchfile) = tempfile_with_mode ($outdir, oct($mode));
     }
 
   my $in = new Autom4te::XFile ($ocache . $req->id, "<");
---