std: fix stack buffer overflow in Windows junction_point by devnexen · Pull Request #158147 · rust-lang/rust

devnexen · 2026-06-19T17:55:49Z

The guard checked data_len > u16::MAX, allowing paths far larger than PathBuffer (a fixed 16384-element array), which the subsequent single copy_from then overflows. Bound against MAXIMUM_REPARSE_DATA_BUFFER_SIZE plus header instead, matching the kernel's limit.

rustbot · 2026-06-19T17:55:56Z

r? @Darksonn

rustbot has assigned @Darksonn.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ChrisDenton, libs
@ChrisDenton, libs expanded to 12 candidates
Random selection from Darksonn, Mark-Simulacrum, clarfonthey, jhpratt

Darksonn

@rustbot author

View changes since this review

Darksonn · 2026-06-19T20:18:58Z

It sounds like this should be changed to ptr[..abs_path.len()].copy_from_slice(abs_path) or similar so that we actually perform a bounds check here.

Also, it would be really nice with a test that'd catch this (may be easier to check that the test is failing if we insert a bounds check first).

In fact, maybe the bounds check should be entirely rewritten to

ptr.get(..abs_path.len()) .ok_or_else(|| io::const_error!(io::ErrorKind::InvalidInput, "`original` path is too long"))? .copy_from_slice(abs_path)

This way there's no risk that the two bounds checks are not kept in sync. For instance, why is there a + 8 in the previous check? I don't understand that part.

Darksonn · 2026-06-19T20:20:34Z

        PathBuffer: [MaybeUninit<u16>; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize],
    }
    let data_len = 12 + (abs_path.len() * 2);
-    if data_len > u16::MAX as usize {
+    if data_len + 8 > c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize {
        return Err(io::const_error!(io::ErrorKind::InvalidInput, "`original` path is too long"));


data_len counts the number of bytes, but MAXIMUM_REPARSE_DATA_BUFFER_SIZE counts the number of u16s. This doesn't sound right.

rustbot · 2026-06-19T20:20:54Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

devnexen · 2026-06-19T21:04:06Z

@rustbot ready

Darksonn

@rustbot author

View changes since this review

Darksonn · 2026-06-19T21:07:53Z


+#[test]
+#[cfg(windows)]
+fn junction_point_overlong_path() {


I'd like to see this regression test fail without the change. Do you mind opening a second temporary PR containing just the test so that we can run it through CI? We can close it again when we've confirmed the regression test catches the bug.

Confirmed failure in #158201.

Darksonn · 2026-06-19T21:11:00Z

-        PathBuffer: [MaybeUninit::uninit(); c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize],
+        PathBuffer: [MaybeUninit::uninit(); c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize / 2],
    };
+    // The path is followed by its null terminator and the (empty) print name's
+    // null terminator, so the buffer must hold two extra `u16`s. This single
+    // bounds check keeps the buffer, the copy, and `data_len` below in sync; if
+    // the path doesn't fit, fail rather than overflow the buffer.
+    let Some(ptr) = header.PathBuffer.get_mut(..abs_path.len() + 2) else {
+        return Err(io::const_error!(io::ErrorKind::InvalidInput, "`original` path is too long"));
+    };
+    ptr[..abs_path.len()].write_copy_of_slice(&abs_path);


This buffer is uninitialized. Don't you need to set path[abs_path.len()] and path[abs_path.len()+1] to zero?

The path has an explicit length so it doesn't need to be null terminated at all. I'm uncertain where that idea came from but it's probably just a misunderstanding? I mean, I guess there's no harm in writing a null but if so it shouldn't be part of the length.

ChrisDenton · 2026-06-19T21:22:30Z

To give some context here, this was originally a hacky function used only in tests. It's currently publicly exposed as a nightly-only API but it's not considered ready for stabilisation. E.g. 16kb is way too much for a stack buffer (though admittedly using a stack buffer for shorter paths would be useful).

ChrisDenton · 2026-06-19T21:35:49Z

-        PathBuffer: [MaybeUninit<u16>; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize],
+        // `MAXIMUM_REPARSE_DATA_BUFFER_SIZE` is a size in bytes; halve it for a
+        // count of `u16`s (the `readlink` path uses it as a byte buffer).
+        PathBuffer: [MaybeUninit<u16>; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize / 2],


This doesn't sound right at all. What makes you say MAXIMUM_REPARSE_DATA_BUFFER_SIZE is in bytes?

View changes since the review

Huh, apparently it is.

The protocol docs make no mention of a maximum size, only that it's a 16 bit unsigned integer (hence my surprise at 16kb being a limit). But the public headers for the kernel API do have MAXIMUM_REPARSE_DATA_BUFFER_SIZE in bytes of 16kb.

Should it be changed to an u8 array as well?

I would say no. It's the mount-point WCHAR path, and u16 keeps the copy a clean write_copy_of_slice(&abs_path).

Temporary CI-verification test for #158147. Without the fix, a >16 KiB junction target passes the old `> u16::MAX` length check yet overflows the inline reparse stack buffer. This test must fail on master and pass once the fix lands.

devnexen · 2026-06-21T11:15:17Z

@rustbot ready

Darksonn

@rustbot author

View changes since this review

Darksonn · 2026-06-22T05:56:05Z

+    // Size of the reparse data after the 8-byte ReparseTag/ReparseDataLength/
+    // Reserved header: the four name offset/length `u16` fields (8 bytes) plus
+    // the path.
+    let data_len = 8 + abs_path.len() * 2;


Isn't it 16 bytes?

ReparseTag is 4 bytes

ReparseDataLength is 2 bytes

Reserved is 2 bytes

SubstituteNameOffset is 2 bytes

SubstituteNameLength is 2 bytes

PrintNameOffset is 2 bytes

PrintNameLength is 2 bytes

Right, it's 16 bytes. Switched to offset_of! so both lengths derive from the struct.

The guard checked `data_len > u16::MAX`, allowing paths far larger than `PathBuffer` (a fixed 16384-element array), which the subsequent single `copy_from` then overflows. Bound against `MAXIMUM_REPARSE_DATA_BUFFER_SIZE` plus header instead, matching the kernel's limit.

devnexen · 2026-06-23T00:07:07Z

@rustbot ready

Darksonn

This looks okay to me. @ChrisDenton do you have any comments before I merge it?

View changes since this review

rustbot assigned Darksonn Jun 19, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 19, 2026

Darksonn reviewed Jun 19, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 19, 2026

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2026

Darksonn reviewed Jun 19, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 19, 2026

ChrisDenton reviewed Jun 19, 2026

View reviewed changes

devnexen force-pushed the windows_fs_oflow_fix branch from d842451 to 28748e5 Compare June 21, 2026 06:05

devnexen mentioned this pull request Jun 21, 2026

std: regression test for Windows junction_point stack buffer overflow #158201

Closed

Darksonn added O-windows Operating system: Windows A-filesystem Area: `std::fs` labels Jun 21, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2026

Darksonn reviewed Jun 22, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2026

devnexen force-pushed the windows_fs_oflow_fix branch from 28748e5 to b7af0d1 Compare June 22, 2026 23:24

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 23, 2026

Darksonn approved these changes Jun 23, 2026

View reviewed changes

Uh oh!

Conversation

devnexen commented Jun 19, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 19, 2026

Uh oh!

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jun 19, 2026

Uh oh!

This comment has been minimized.

devnexen commented Jun 19, 2026

Uh oh!

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisDenton Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisDenton commented Jun 19, 2026

Uh oh!

ChrisDenton Jun 19, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnexen commented Jun 21, 2026

Uh oh!

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnexen commented Jun 23, 2026

Uh oh!

Darksonn left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

devnexen commented Jun 19, 2026 •

edited by rustbot

Loading

Darksonn left a comment •

edited by rustbot

Loading

Darksonn left a comment •

edited by rustbot

Loading

ChrisDenton Jun 19, 2026 •

edited

Loading

ChrisDenton Jun 19, 2026 •

edited by rustbot

Loading

Darksonn left a comment •

edited by rustbot

Loading

Darksonn left a comment •

edited by rustbot

Loading