chroot_realpath and tests: Replace sprintf with snprintf for security #1885

osamakader · 2025-09-26T20:55:04Z

Replace all instances of sprintf with snprintf to prevent buffer overflows. This follows the security hardening mentioned in NEWS.md where sprintf was replaced with safer alternatives.

Changes made:

tests/init.c: Replace sprintf with snprintf for proc paths
tests/tests_libcrun_fuzzer.c: Replace sprintf with snprintf for ID generation
src/libcrun/chroot_realpath.c: Replace sprintf with snprintf for path resolution

All changes use appropriate buffer size limits to prevent overflow.

Summary by Sourcery

Harden string formatting across the project by replacing unsafe sprintf calls with bounded snprintf usages and explicit buffer size limits to prevent buffer overflows in both test code and core functionality.

Enhancements:

Replace sprintf with snprintf for proc paths in tests/init.c
Replace sprintf with snprintf for fuzzer ID generation in tests/tests_libcrun_fuzzer.c
Replace sprintf with snprintf for path resolution in src/libcrun/chroot_realpath.c

sourcery-ai · 2025-09-26T20:55:10Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replace all unsafe sprintf calls with snprintf using fixed buffer limits (sizeof buffers or PATH_MAX) across test initialization, fuzzer ID generation, and core chroot_realpath logic to mitigate potential buffer overflow vulnerabilities.

File-Level Changes

Change	Details	Files
Bound path formatting in tests/init.c by replacing sprintf with snprintf to prevent overflows	Replace sprintf for proc_path uid_map formatting Replace sprintf for proc_path gid_map formatting Replace sprintf for proc_path user namespace formatting Replace sprintf for cgroup directory path creation Replace sprintf for cgroup procs file path formatting	`tests/init.c`
Use snprintf for fuzzer ID generation in tests/tests_libcrun_fuzzer.c	Replace sprintf with snprintf for id buffer formatting	`tests/tests_libcrun_fuzzer.c`
Harden chroot_realpath logic with snprintf to limit resolved path length	Replace sprintf with snprintf using PATH_MAX in error-handling output	`src/libcrun/chroot_realpath.c`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey there - I've reviewed your changes - here's some feedback:

In chroot_realpath.c, consider replacing the hard-coded PATH_MAX with sizeof(resolved_path) or an explicit bufsize parameter to ensure snprintf’s size matches the actual buffer length.
The change to the HF_ITER extern declaration in tests_libcrun_fuzzer.c looks like an unrelated formatting tweak—please revert or separate it into its own PR to keep the snprintf changes focused.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In chroot_realpath.c, consider replacing the hard-coded PATH_MAX with sizeof(resolved_path) or an explicit bufsize parameter to ensure snprintf’s size matches the actual buffer length.
- The change to the HF_ITER extern declaration in tests_libcrun_fuzzer.c looks like an unrelated formatting tweak—please revert or separate it into its own PR to keep the snprintf changes focused.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

packit-as-a-service · 2025-09-26T21:04:31Z

Ephemeral COPR build failed. @containers/packit-build please check.

kolyshkin

The first commit says "tests:" in the subject line but modifies src/libcrun/chroot_realpath.c.

In general, we are not interested in hardening tests security.

kolyshkin · 2025-09-27T00:44:05Z

tests/tests_libcrun_fuzzer.c

    }
 #ifdef FUZZER
-  extern void HF_ITER (uint8_t **buf, size_t *len);
+  extern void HF_ITER (uint8_t * *buf, size_t * len);


What is this for?

kolyshkin · 2025-09-27T00:44:25Z

tests/init.c

    {
      /* change one page each 0.1 seconds */
-      nanosleep ((const struct timespec[]) { { 0, 100000000L } }, NULL);
+      nanosleep ((const struct timespec[]){ { 0, 100000000L } }, NULL);


What is this for?

kolyshkin

OK, this is really hard to review since you are modifying the single line in chroot_realpath.c in three subsequent commits. Marking this as draft for now; once ready for review, please click to "ready for review".

sourcery-ai

Hey there - I've reviewed your changes - here's some feedback:

In chroot_realpath, verify snprintf’s return value to detect and handle any potential buffer truncation instead of assuming full writes.
Prefer using sizeof(resolved_path) (the actual buffer length) instead of hardcoded PATH_MAX to keep snprintf size in sync with the buffer.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In chroot_realpath, verify snprintf’s return value to detect and handle any potential buffer truncation instead of assuming full writes.
- Prefer using sizeof(resolved_path) (the actual buffer length) instead of hardcoded PATH_MAX to keep snprintf size in sync with the buffer.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

osamakader · 2025-09-27T11:45:59Z

@kolyshkin I forgot to rebase the commits, now should be ok. Please re-review.
Thanks.

Replace sprintf with snprintf to prevent buffer overflows in chroot_realpath.c. This follows the security hardening mentioned in NEWS.md where sprintf was replaced with safer alternatives. The function uses PATH_MAX as the buffer size since sizeof on array function parameters returns the pointer size, not the actual buffer size. Signed-off-by: Osama Abdelkader <[email protected]>

Replace sprintf with snprintf in test files to prevent buffer overflows. This follows the security hardening mentioned in NEWS.md where sprintf was replaced with safer alternatives. Changes made: - tests/init.c: Replace sprintf with snprintf for proc paths - tests/tests_libcrun_fuzzer.c: Replace sprintf with snprintf for ID generation Signed-off-by: Osama Abdelkader <[email protected]>

sourcery-ai bot approved these changes Sep 26, 2025

View reviewed changes

kolyshkin reviewed Sep 27, 2025

View reviewed changes

kolyshkin marked this pull request as draft September 27, 2025 00:46

osamakader force-pushed the sprintf-to-snprintf branch 2 times, most recently from b302be8 to 11a40b1 Compare September 27, 2025 11:43

osamakader marked this pull request as ready for review September 27, 2025 11:44

osamakader requested a review from kolyshkin September 27, 2025 11:45

sourcery-ai bot approved these changes Sep 27, 2025

View reviewed changes

osamakader changed the title ~~tests: Replace sprintf with snprintf for security~~ chroot_realpath and tests: Replace sprintf with snprintf for security Sep 27, 2025

osamakader added 2 commits September 27, 2025 14:56

osamakader force-pushed the sprintf-to-snprintf branch from 11a40b1 to ccbf0d9 Compare September 27, 2025 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chroot_realpath and tests: Replace sprintf with snprintf for security #1885

chroot_realpath and tests: Replace sprintf with snprintf for security #1885

osamakader commented Sep 26, 2025 •

edited by sourcery-ai bot

Loading

Uh oh!

sourcery-ai bot commented Sep 26, 2025 •

edited

Loading

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Uh oh!

packit-as-a-service bot commented Sep 26, 2025

Uh oh!

kolyshkin left a comment

Uh oh!

kolyshkin Sep 27, 2025

Uh oh!

kolyshkin Sep 27, 2025

Uh oh!

kolyshkin left a comment

Uh oh!

sourcery-ai bot left a comment

Uh oh!

osamakader commented Sep 27, 2025

Uh oh!

Uh oh!

chroot_realpath and tests: Replace sprintf with snprintf for security #1885

Are you sure you want to change the base?

chroot_realpath and tests: Replace sprintf with snprintf for security #1885

Conversation

osamakader commented Sep 26, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

packit-as-a-service bot commented Sep 26, 2025

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

kolyshkin Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

kolyshkin Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

osamakader commented Sep 27, 2025

Uh oh!

Uh oh!

osamakader commented Sep 26, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Sep 26, 2025 •

edited

Loading