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, "<"); ---