[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHhgV8hQR51pP=ioqw8Q2YFCcTZUOs7JaQv8Wq1gW=r2PyKP-A@mail.gmail.com>
Date: Mon, 2 Jun 2025 19:22:49 +0200
From: Leon Timmermans <fawaka@...il.com>
To: Florian Weimer <fweimer@...hat.com>
Cc: Stig Palmquist <stig@...g.io>, perl5-porters@...l.org, oss-security@...ts.openwall.com
Subject: Re: CVE-2025-40909: Perl threads have a working directory race
condition where file operations may target unintended paths
On Mon, Jun 2, 2025 at 10:22 AM Florian Weimer via perl5-porters
<perl5-porters@...l.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
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
