Re: CVE-2025-40909: Perl threads have a working directory race condition where file operations may target unintended paths

[Leon Timmermans <fawaka () gmail com>]

On Mon, Jun 2, 2025 at 10:22 AM Florian Weimer via perl5-porters
<perl5-porters () perl org> wrote:
> * Stig Palmquist:
>
> > References
> > ----------
> > https://github.com/Perl/perl5/commit/918bfff86ca8d6d4e4ec5b30994451e0bd74aba9.patch
>
> Is this fix really correct?
>
> +    ret = fdopendir(dup(my_dirfd(dp)));
>
> This does not create a separate open file description, only a second
> descriptor that shares the read position of the directory stream with
> the original directory stream.  I think you have to use something like
> this:
>
>      ret = fdopendir(openat(my_dirfd(dp), ".", O_DIRECTORY | O_CLOEXEC));

Our thread cloning in general is a terribly awkward business, where
"what is the correct behavior" isn't always well defined or possible;
I can see the arguments for both to be honest.

For file descriptors we don't create new file descriptions either (we
don't even create new file descriptors, we refcount them), so why
should we do so for directory handles? I'm not sure that expectation
makes sense in that context.

And if we did go the openat way, I don't think that seekdir on the new
handle with the telldir of the old one is necessarily valid if the
directory has been changed (I mean even a rewinddir can invalidate
telldir's return value). I don't think we can do a fully correct copy
here.

> (The original dup approach failed to set the O_CLOEXEC flag, potentially
> causing the descriptor to leak to subprocesses.)

You're correct, I should have used PerlLIO_dup_cloexec there.

> Futhermore, if there is error reporting using errno in the Perl code (I
> haven't checked), it makes sense not to pass a -1 failure indicator from
> openat to fdopendir because that unconditionally results in EBADF
> instead of more precise error codes such as ENFILE or EMFILE.

Yeah that makes sense.

Leon