Stack buffer overflow in prepare_input_tensors() due to unchecked memcpy size #16797

lucylq · 2026-01-22T21:20:02Z

Summary

Add size checks for int, double, bool before memcpying. Otherwise the input buffer may be larger than the intended size and overwrite adjacent memory.

Test Plan

(executorch) [[email protected] /data/users/lfq/executorch/build (lfq.check-size-before-memcpy)]$ ctest -R extension_runner_util_test -V
...
54: [ RUN      ] InputsTest.DoubleInputWrongSizeFails
54: E 00:00:00.001251 executorch:inputs.cpp:101] Double input at index 2 has size 16, expected sizeof(double) 8
54: [       OK ] InputsTest.DoubleInputWrongSizeFails (0 ms)
...

1/1 Test #54: extension_runner_util_test .......   Passed    0.01 sec

The following tests passed:
        extension_runner_util_test

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   0.01 sec

pytorch-bot · 2026-01-22T21:20:07Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16797

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit 9c75403 with merge base 86b4bea ():

NEW FAILURE - The following job has failed:

Test CUDA Builds / export-model-cuda-artifact (openai, whisper-large-v3-turbo, non-quantized) / linux-job (gh)
RuntimeError: Command docker exec -t 8efeb32983fc0e56aebfb79b4d566ab1a19f2a69d35db8b22cf7b3fec6dfd599 /exec failed with exit code 1

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2026-01-22T21:21:00Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copilot

Pull request overview

Mitigates a stack buffer overflow risk in prepare_input_tensors() by validating scalar input buffer sizes before copying into stack-allocated variables.

Changes:

Add strict size checks for Tag::Int (int64_t), Tag::Double (double), and Tag::Bool (bool) inputs prior to memcpy.
Return InvalidArgument with a descriptive error when the provided scalar buffer size is unexpected.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

extension/runner_util/inputs.cpp

mergennachin · 2026-01-22T22:04:29Z

Can you add a regression test?

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

extension/runner_util/inputs.cpp

extension/runner_util/test/inputs_test.cpp

extension/runner_util/inputs.cpp

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

extension/runner_util/inputs.cpp

lucylq · 2026-01-23T18:42:57Z

Can you add a regression test?

@mergennachin added one for double. Feel like that should be OK as it's the same logic, and adding tests for bool/int would need new models to export_program.py that take in bool/int as input. But let me know what you think

extension/runner_util/inputs.cpp

larryliu0820

Left some non-blocking comments

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

extension/runner_util/test/CMakeLists.txt

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

extension/runner_util/test/inputs_test.cpp

Copilot · 2026-01-27T23:31:37Z

extension/runner_util/test/inputs_test.cpp

+    const char* add_path = std::getenv("ET_MODULE_ADD_PATH");
+    Result<FileDataLoader> add_loader = FileDataLoader::from(add_path);
+    ASSERT_EQ(add_loader.error(), Error::Ok);
+    add_loader_ = std::make_unique<FileDataLoader>(std::move(add_loader.get()));
+
+    Result<Program> add_program = Program::load(
+        add_loader_.get(), Program::Verification::InternalConsistency);
+    ASSERT_EQ(add_program.error(), Error::Ok);
+    add_program_ = std::make_unique<Program>(std::move(add_program.get()));
+
+    add_mmm_ = std::make_unique<ManagedMemoryManager>(
+        /*planned_memory_bytes=*/32 * 1024U,
+        /*method_allocator_bytes=*/32 * 1024U);
+
+    Result<Method> add_method =
+        add_program_->load_method("forward", &add_mmm_->get());
+    ASSERT_EQ(add_method.error(), Error::Ok);
+    add_method_ = std::make_unique<Method>(std::move(add_method.get()));
+
+    // Load ModuleIntBool
+    const char* intbool_path = std::getenv("ET_MODULE_INTBOOL_PATH");
+    Result<FileDataLoader> intbool_loader = FileDataLoader::from(intbool_path);
+    ASSERT_EQ(intbool_loader.error(), Error::Ok);


SetUp() assumes the environment variables are present. If ET_MODULE_ADD_PATH/ET_MODULE_INTBOOL_PATH are missing, the failure will show up later as a generic FileDataLoader::from() error. Adding ASSERT_NE(getenv(...), nullptr) for each path would make failures much easier to diagnose.

think it's probably fine to have the filedataloader error show up.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T01:37:59Z

extension/runner_util/test/inputs_test.cpp

+  // Double is size 8; use a larger buffer to invoke overflow.
+  char large_buffer[16];
+  memcpy(large_buffer, &alpha, sizeof(double));
+


This file now calls memcpy() directly; consider adding an explicit #include <cstring> (and #include <cstdlib> for std::getenv) to avoid relying on transitive includes for C library declarations.

Copilot · 2026-01-28T01:37:59Z

extension/runner_util/test/inputs_test.cpp

+  {
+    std::vector<std::pair<char*, size_t>> input_buffers;
+
+    char int_buffer[8];


Avoid the magic constant 8 for the int buffer size; use sizeof(int64_t) (or sizeof(y)) so the test stays consistent with the production check (buffer_size != sizeof(int64_t)).

Suggested change

char int_buffer[8];

char int_buffer[sizeof(int64_t)];

Copilot · 2026-01-28T01:37:59Z

extension/runner_util/test/inputs_test.cpp

+    const char* add_path = std::getenv("ET_MODULE_ADD_PATH");
+    Result<FileDataLoader> add_loader = FileDataLoader::from(add_path);
+    ASSERT_EQ(add_loader.error(), Error::Ok);


std::getenv("ET_MODULE_ADD_PATH") can return nullptr; passing that into FileDataLoader::from() just yields a generic "File name cannot be empty" failure. Add an ASSERT_NE(add_path, nullptr) (ideally mentioning the env var name) before calling FileDataLoader::from() to make misconfigured test environments easier to debug.

Copilot · 2026-01-28T01:37:59Z

extension/runner_util/test/inputs_test.cpp

+    add_method_ = std::make_unique<Method>(std::move(add_method.get()));
+
+    // Load ModuleIntBool
+    const char* intbool_path = std::getenv("ET_MODULE_INTBOOL_PATH");


Same as above for ET_MODULE_INTBOOL_PATH: add an ASSERT_NE(intbool_path, nullptr) (with a helpful message) before calling FileDataLoader::from() so failures clearly indicate a missing env var.

Suggested change

const char* intbool_path = std::getenv("ET_MODULE_INTBOOL_PATH");

const char* intbool_path = std::getenv("ET_MODULE_INTBOOL_PATH");

ASSERT_NE(intbool_path, nullptr)

<< "ET_MODULE_INTBOOL_PATH environment variable must be set";

meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 22, 2026

lucylq marked this pull request as ready for review January 22, 2026 21:22

lucylq requested review from JacobSzwejbka, Copilot and mergennachin January 22, 2026 21:22

Copilot started reviewing on behalf of lucylq January 22, 2026 21:22 View session

lucylq added the security-fix label Jan 22, 2026

Copilot AI reviewed Jan 22, 2026

View reviewed changes

extension/runner_util/inputs.cpp Outdated Show resolved Hide resolved

extension/runner_util/inputs.cpp Outdated Show resolved Hide resolved

lucylq force-pushed the lfq.check-size-before-memcpy branch from b0e5c6d to 8c8d7dc Compare January 23, 2026 00:52

lucylq requested a review from Copilot January 23, 2026 00:55

Copilot started reviewing on behalf of lucylq January 23, 2026 00:56 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

lucylq force-pushed the lfq.check-size-before-memcpy branch from 8c8d7dc to c621f13 Compare January 23, 2026 01:13

Copilot AI review requested due to automatic review settings January 23, 2026 01:14

lucylq force-pushed the lfq.check-size-before-memcpy branch from c621f13 to 33aa861 Compare January 23, 2026 01:14

Copilot started reviewing on behalf of lucylq January 23, 2026 01:14 View session

lucylq force-pushed the lfq.check-size-before-memcpy branch from 33aa861 to 8090b55 Compare January 23, 2026 01:14

Copilot AI reviewed Jan 23, 2026

View reviewed changes

extension/runner_util/inputs.cpp Show resolved Hide resolved

larryliu0820 reviewed Jan 27, 2026

View reviewed changes

extension/runner_util/inputs.cpp Show resolved Hide resolved

larryliu0820 approved these changes Jan 27, 2026

View reviewed changes

lucylq force-pushed the lfq.check-size-before-memcpy branch from 8090b55 to 801f5a3 Compare January 27, 2026 23:01

lucylq requested a review from kirklandsign as a code owner January 27, 2026 23:01

lucylq requested a review from Copilot January 27, 2026 23:04

Copilot started reviewing on behalf of lucylq January 27, 2026 23:05 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

extension/runner_util/test/CMakeLists.txt Show resolved Hide resolved

lucylq force-pushed the lfq.check-size-before-memcpy branch from 801f5a3 to e95bc65 Compare January 27, 2026 23:23

lucylq requested a review from Copilot January 27, 2026 23:24

Copilot started reviewing on behalf of lucylq January 27, 2026 23:24 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

lucylq force-pushed the lfq.check-size-before-memcpy branch from e95bc65 to 1f63405 Compare January 28, 2026 01:26

Check size before memcpy

9c75403

Copilot AI review requested due to automatic review settings January 28, 2026 01:31

lucylq force-pushed the lfq.check-size-before-memcpy branch from 1f63405 to 9c75403 Compare January 28, 2026 01:31

Copilot started reviewing on behalf of lucylq January 28, 2026 01:32 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Stack buffer overflow in prepare_input_tensors() due to unchecked memcpy size #16797

Are you sure you want to change the base?

Stack buffer overflow in prepare_input_tensors() due to unchecked memcpy size #16797

Conversation

lucylq commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

pytorch-bot bot commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16797

❌ 1 New Failure

Uh oh!

github-actions bot commented Jan 22, 2026

This PR needs a release notes: label

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

mergennachin commented Jan 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

lucylq commented Jan 23, 2026

Uh oh!

Uh oh!

larryliu0820 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

lucylq Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

lucylq commented Jan 22, 2026 •

edited

Loading

pytorch-bot bot commented Jan 22, 2026 •

edited

Loading

This PR needs a `release notes:` label