Avoid squirrelly `memcpy()` call in `filesystem.cpp` #4933

StephanTLavavej · 2024-09-04T05:21:25Z

We have an extremely squirrelly line of code that's memcpying two consecutive DWORDs into the beginning of a buffer. There's no reason for this weirdness - it isn't perf-critical, and the optimizer should understand memcpy. Now, code analysis tools (specifically CodeQL) are noticing that this code is a 🐿️ read overrun. Let's avoid this by splitting it up into two separate reads.

For the destination, _Id points to FILE_ID_INFO. Its FileId is FILE_ID_128, which contains BYTE Identifier[16];.

For the source, _Info is BY_HANDLE_FILE_INFORMATION:

typedef struct _BY_HANDLE_FILE_INFORMATION {
  DWORD    dwFileAttributes;
  FILETIME ftCreationTime;
  FILETIME ftLastAccessTime;
  FILETIME ftLastWriteTime;
  DWORD    dwVolumeSerialNumber;
  DWORD    nFileSizeHigh;
  DWORD    nFileSizeLow;
  DWORD    nNumberOfLinks;
  DWORD    nFileIndexHigh;
  DWORD    nFileIndexLow;
} BY_HANDLE_FILE_INFORMATION, *PBY_HANDLE_FILE_INFORMATION, *LPBY_HANDLE_FILE_INFORMATION;

StephanTLavavej · 2024-09-09T07:02:16Z

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

ariccio · 2025-03-16T17:45:52Z

(sorry to resurrect an obscure old issue & pr)

Do you or anybody else remember what codeql query was responsible for detecting this? I'm catching up on all that I've missed in MSVC for the past 5-6 years, and this piqued my interest. I'm super glad to see that tools have kept improving!

I remember back in the day when working on tools to account for disk space usage that some of these APIs made me a bit nervous and static analysis wasn't quite up to it yet.

StephanTLavavej · 2025-03-16T22:22:37Z

It was a cpp/overflow-buffer warning SM01947 "Call to memory access function may overflow buffer" (filed as internal VSO-2255048).

Avoid squirrelly memcpy calls

b359606

StephanTLavavej added enhancement Something can be improved filesystem C++17 filesystem labels Sep 4, 2024

StephanTLavavej requested a review from a team as a code owner September 4, 2024 05:21

StephanTLavavej mentioned this pull request Sep 4, 2024

Maintainer priorities #4700

Open

CaseyCarter approved these changes Sep 4, 2024

View reviewed changes

StephanTLavavej self-assigned this Sep 9, 2024

StephanTLavavej merged commit 479dcb9 into microsoft:main Sep 9, 2024
39 checks passed

StephanTLavavej deleted the the-unbeatable-squirrel-filesystem branch September 9, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid squirrelly `memcpy()` call in `filesystem.cpp` #4933

Avoid squirrelly `memcpy()` call in `filesystem.cpp` #4933

Uh oh!

StephanTLavavej commented Sep 4, 2024

Uh oh!

StephanTLavavej commented Sep 9, 2024

Uh oh!

Uh oh!

ariccio commented Mar 16, 2025

Uh oh!

StephanTLavavej commented Mar 16, 2025

Uh oh!

Uh oh!

Avoid squirrelly memcpy() call in filesystem.cpp #4933

Avoid squirrelly memcpy() call in filesystem.cpp #4933

Uh oh!

Conversation

StephanTLavavej commented Sep 4, 2024

Uh oh!

StephanTLavavej commented Sep 9, 2024

Uh oh!

Uh oh!

ariccio commented Mar 16, 2025

Uh oh!

StephanTLavavej commented Mar 16, 2025

Uh oh!

Uh oh!

Avoid squirrelly `memcpy()` call in `filesystem.cpp` #4933

Avoid squirrelly `memcpy()` call in `filesystem.cpp` #4933