Re^4: Insecure dependency in mkdir while running with -T switch at ... File/Temp.pm line 542

I got the first results back from an rc4b-TRIAL that tries out hy's suggestion

  my $tname = shift;
  $tname = untaint_var($tname);
...
  my $abslogdir = File::Spec->canonpath(Cwd::realpath("log"));
  $abslogdir  = untaint_var($abslogdir);
  $workdir = File::Spec->abs2rel(tempdir("$tname.XXXXXX", DIR => $absl
+ogdir));
[download]

Same failure.

I've also earlier uploaded an rc4c-TRIAL that is the same as rc4b-TRIAL except adding a module dependency on File::Temp so the report will show what version it loads, and should see results from that some hours from now.

I looked at the code in File::Temp and File::Spec, particularly File::Spec->splitpath, File::Spec->splitdir and File::Spec->catdir that hy pointed out, and I don't see anything it does that could taint the $path variable that is eventually passed to that mkdir on line 542 when, as in this case, tempdir is called with just template and DIR arguments that are not tainted.

If there are no other hints, I think I will just have to leave it as something on BINGOS' 64 bit test runners (doesn't happen on their i386 and i686 architecture runners) that I can't reproduce, and ignore it unless some user of our module reports it trying to install it.

Ken, thanks for trying it out. The optional modules are optional for a reason. I know some are not really maintained and either don't load or are finicky to load on Windows, like the three you mentioned. They still work on enough platforms that we make use of them if they are available.

Comment on Re^4: Insecure dependency in mkdir while running with -T switch at ... File/Temp.pm line 542 Download Code

Replies are listed 'Best First'.
Re^5: Insecure dependency in mkdir while running with -T switch at ... File/Temp.pm line 542 by hv (Prior) on Dec 11, 2022 at 02:15 UTC
hy `s/hy/hv/g; # :)` Same failure. Ah well, it was worth a try. (I had not appreciated that the sample fail was on Linux, I had imagined more esoteric systems.) I've looked at more code, and am not seeing any obvious opportunities to get tainted results. I could just about imagine getting taint interaction from `rand()` - at least, I know that some systems provide device sources of randomness, which would involve interacting with the filesystem. I'm not aware of any way to configure perl to use those directly with `rand()` though, and I don't see anything in perl source that would touch a filesystem or environment variables if the standard `pp_rand()` gets called. I can't rule out something replacing `pp_rand` in `PL_ppaddr[]` though. All I can really suggest is to get more instrumentation by shipping a copy of (some version of) `File::Temp::tempdir()` (and/or a monkey-patched `File::Temp::_gettemp()`) with checks at various points that `$path` is touched. I see that Scalar::Util provides `tainted()` at least as far back as the version shipped with perl-5.8, so that's probably a reasonable way to check. The other option would be to contact BINGOS and ask if he could investigate, or give you access to one of the affected systems. And - long shot - it would also be interesting to check if there are any other modules that both use <c>tempdir() and test its behaviour under tainting that have also been tested by the same smokers.	[reply] [d/l] [select]
Re^6: Insecure dependency in mkdir while running with -T switch at ... File/Temp.pm line 542 by sidney (Acolyte) on Dec 11, 2022 at 21:05 UTC
hv :) I uploaded a renamed copy of rc3 as rc3u and verified that the same test runners that BINGOS is running that fail on rc4 fail on that too, and the failure did not happen on BINGOS' runners with the same platform and perl version when they ran rc3 last September. The details in the reports that passed in September and failed now show no difference in the perl compile options nor in the output from uname for the OS. It only happens on the the 5.14.x, 5.18.x, and 5.20.x runners and nobody else's runners with those perl versions. The 64-bit/32-bit architecture difference is a red herring: He doesn't have any 32-bit runners with that old perl versions anyway. The failures still happen with an rc4c-TRIAL I uploaded which has code ensuring that what is passed to tempdir is untainted and uses an absolute path so can't be a Cwd issue, and which shows the File::Temp version is 0.2311 and the File::Spec version is 3.75. At this point I think I will just ignore those errors unless someone encounters it who can give me access to their machine or a hint that lets me reproduce the problem. I have emailed BINGOS, but I don't think it is worth worrying about anymore. Worst case if someone encounters this in production I can suggest they upgrade their perl to something newer than 5.20.	[reply]