Skip to content

Harden fm-shim-backend URI length handling#2

Open
assisted-by-ai wants to merge 1 commit into
masterfrom
codex/review-and-fix-memory-safety-issues
Open

Harden fm-shim-backend URI length handling#2
assisted-by-ai wants to merge 1 commit into
masterfrom
codex/review-and-fix-memory-safety-issues

Conversation

@assisted-by-ai
Copy link
Copy Markdown
Owner

Motivation

  • Prevent signed-to-unsigned conversion issues and oversized allocations when handling URI lists from D-Bus which could lead to memory-safety problems.

Description

  • Changed launch_frontend_process signature to accept size_t uri_list_len so allocation/index math is size-safe.
  • Added an explicit overflow guard if (uri_list_len > SIZE_MAX - 4) before allocating the argument array for execve().
  • Validated the int uri_list_len returned by dbus_message_get_args to detect and reject negative lengths before converting to size_t.
  • Propagated the validated size_t length into both the no-reply and normal reply code paths when calling launch_frontend_process.

Testing

  • Ran syntax/quick checks with gcc -fsyntax-only and gcc -x c -fsyntax-only to validate the changes; these checks failed to complete a full build because the container lacks dbus-1/libsystemd development headers and pkg-config metadata, so a full compile/link test could not be performed.
  • No unit tests are present; changes were kept minimal and focused on type/overflow checking to limit behavioral impact.

Codex Task

@assisted-by-ai
Copy link
Copy Markdown
Owner Author

Summary
Reviewed fm-shim-backend.c for memory-safety issues around URI count handling and fixed an unsafe signed→unsigned path by changing launch_frontend_process() to accept size_t for URI list length. This removes repeated casts and keeps indexing/allocation arithmetic type-safe.

Added an explicit overflow guard before building arg_arr (uri_list_len + 4) to prevent wraparound in allocation sizing for execve() arguments.

Added validation for negative uri_list_len returned from D-Bus argument decoding; invalid negative lengths are now rejected before conversion, preventing huge size_t values and potential oversized allocations/out-of-bounds behavior.

Updated both call paths to pass the validated size_t length into launch_frontend_process() consistently (no-reply and normal reply flow).

@ArrayBolt3
Copy link
Copy Markdown

ArrayBolt3 commented Mar 15, 2026

Integrated the useful changes from this in ArrayBolt3@c96fb11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants