IssueTracker

Hello, and thanks! We acknowledge receiving the report.

For the Security Group, how do we want to handle this? GCC is planning a fix in a public issue [1], and the fact libc++ is vulnerable has been discussed in a lot of public venues [2] [3] [4] [5]. I'm not sure how worth it is to keep this issue private since this problem is basically public knowledge by now.

Pietro.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104161
[2] https://www.reddit.com/r/cpp/comments/s8ok0h/possible_toctou_vulnerabilities_in/
[3] https://www.reddit.com/r/rust/comments/s8tvta/comment/htjxkve/
[4] https://www.reddit.com/r/rust/comments/s8h1kr/security_advisory_for_the_standard_library/hth7go5/
[5] https://news.ycombinator.com/item?id=30024983

Pietro, thanks for jumping on this. Agree that, given this is a very public and well-discussed bug, it makes sense to unrestrict and track the fix as a more typical LLVM bug. Will offer a weekday for folks to disagree, then suggest unrestricting EOD on Monday January 24.

Just one business day for disagreements seems a bit short (given the processes discussion we had in the December call). What about giving a week for disagreements for unrestricting this issue, while looping some libc++ maintainers in to start developing a fix (if they so choose) right now? Starting to fix this doesn't require the issue to be public.

[Empty comment from Monorail migration]

Hi folks,

Incidentally, I tried creating an issue that would have been a duplicate of this one but got some sort of 503 error doing so. Anyways, here goes:

I was contacted privately yesterday (2022-01-21) regarding this security vulnerability. It was suggested that this might affect the C++ Standard Library as well, which is very similar in how it handles std::filesystem::remove_all. And indeed, I was able to reproduce the issue with libc++'s std::filesystem::remove_all function by translating the Rust reproducer to C++ (roughly). At this moment, I also have a patch which I think will fix the issue -- it has been tested on Apple platforms.

I am not a security expert, and I would like to get some support in reviewing both the reproducer and the proposed patch to make sure I'm not just monkeying around. I attached the patch in case someone wants to take a look (please do).

FWIW, reproducing this issue with the current version of libc++ is most easily done by adding `std::this_thread::sleep_for(std::chrono::milliseconds(1))` between the time where we check `is_directory` and the time we create the `directory_iterator`. That of course doesn't apply anymore with the suggested patch.

I am also in contact with the GCC folks and the MSVC folks -- I shared this patch with a trusted set of individuals. We believe this doesn't apply to MSVC, but it does apply to GCC. As mentioned above, it looks like GCC is already handling that publicly.

Please let me know how to handle this from here.

re https://crbug.com/llvm/19#c3 -- week sounds great, sorry for the noise.

We're tracking this internally as rdar://87912416

@ldionne: Although I'm by no mean an expert in the field, I've reviewed the patch and the logic makes sense to me. I've (mentally) tried to find race conditions and couldn't find any. One side note though: from the name (and because of how the attack works), I first thought that `__disallow_symlink_root`  had something to do with the `root` user. Maybe add a comment on the enum field or rename it to something something more explicit (weak suggestion: `__disallow_root_directory_symlink`) ?

So what's up? Do I create a normal Phabricator review to fix this, or is there some other process? It looks like the cat is already out of the bag because the bug is public on the GCC side and there's a reddit thread about it here: https://www.reddit.com/r/cpp/comments/s8ok0h/possible_toctou_vulnerabilities_in/.

+1 for the phabricator review, that's what we did for the BiDi patch once the CVE got public.

Given that the problem is already publicized, I don't have any objection to a Phabricator review.

No objection for a Phabricator review either.

The first patch was too naive, I uploaded an entirely new version. Please take a look if you can.

The patch was reviewed and merged, I'm marking the bug as fixed. Thanks all for your involvement here!

Also, it has been (way) more than a week without objections, so I'm unrestricting the issue.

This issue was migrated from crbug.com/llvm/19?no_tracker_redirect=1

[Auto-CCs applied]