Skip to content

Potentially overflowing call to snprintf (alerts 11, 12)#3750

Merged
rhc54 merged 1 commit intoopenpmix:masterfrom
rhc54:topic/fix-alert-11
Dec 25, 2025
Merged

Potentially overflowing call to snprintf (alerts 11, 12)#3750
rhc54 merged 1 commit intoopenpmix:masterfrom
rhc54:topic/fix-alert-11

Conversation

@rhc54
Copy link
Contributor

@rhc54 rhc54 commented Dec 25, 2025

Per Copilot autofix:

In general, when using snprintf in a loop to build up a string, you must check each call’s return value before using it: if the return value is negative or greater than or equal to the remaining buffer size, you should stop writing and handle the error, rather than unconditionally adding the return value to your running length. This prevents len from being corrupted by a “would-have-written” size that is larger than the remaining space.

For this specific function, the safest fix with minimal functional change is:

Track the remaining space explicitly (remaining = max_ranks_len;) and always pass remaining as the size argument.
After each snprintf, if n < 0 or n >= remaining, treat it as failure: free the buffer, set *ranks = NULL, optionally log the error, and return early.
Only if n is valid, subtract it from remaining and increment len by n.
Because we are now checking after each call, the final if (len >= max_ranks_len - 1) becomes unnecessary, but we can keep a light sanity check if desired. To minimize changes, we can retain len and the final check, but make sure we never let len exceed max_ranks_len during the loop.

Concretely, in fill_seq_ranks_array in test/test_server.c, we will:

Introduce an int n; and a int remaining = max_ranks_len;.
Replace the two snprintf calls to:
    Call snprintf(*ranks + len, remaining, ...).
    Check if (n < 0 || n >= remaining) { free(*ranks); *ranks = NULL; TEST_ERROR(...); return; }.
    Update len += n; remaining -= n;.
Keep allocations and the if (0 >= nprocs) early-return logic unchanged.

No new methods or external libraries are required; we only reuse malloc, free, snprintf, and the existing TEST_ERROR macro.

Per Copilot autofix:

In general, when using snprintf in a loop to build up a string, you must check each call’s return value before using it: if the return value is negative or greater than or equal to the remaining buffer size, you should stop writing and handle the error, rather than unconditionally adding the return value to your running length. This prevents len from being corrupted by a “would-have-written” size that is larger than the remaining space.

For this specific function, the safest fix with minimal functional change is:

    Track the remaining space explicitly (remaining = max_ranks_len;) and always pass remaining as the size argument.
    After each snprintf, if n < 0 or n >= remaining, treat it as failure: free the buffer, set *ranks = NULL, optionally log the error, and return early.
    Only if n is valid, subtract it from remaining and increment len by n.
    Because we are now checking after each call, the final if (len >= max_ranks_len - 1) becomes unnecessary, but we can keep a light sanity check if desired. To minimize changes, we can retain len and the final check, but make sure we never let len exceed max_ranks_len during the loop.

Concretely, in fill_seq_ranks_array in test/test_server.c, we will:

    Introduce an int n; and a int remaining = max_ranks_len;.
    Replace the two snprintf calls to:
        Call snprintf(*ranks + len, remaining, ...).
        Check if (n < 0 || n >= remaining) { free(*ranks); *ranks = NULL; TEST_ERROR(...); return; }.
        Update len += n; remaining -= n;.
    Keep allocations and the if (0 >= nprocs) early-return logic unchanged.

No new methods or external libraries are required; we only reuse malloc, free, snprintf, and the existing TEST_ERROR macro.

Signed-off-by: Ralph Castain <rhc@pmix.org>
@rhc54 rhc54 changed the title Potentially overflowing call to snprintf (alert 11) Potentially overflowing call to snprintf (alerts 11, 17) Dec 25, 2025
@rhc54 rhc54 changed the title Potentially overflowing call to snprintf (alerts 11, 17) Potentially overflowing call to snprintf (alerts 11, 12) Dec 25, 2025
@rhc54 rhc54 merged commit b591fcb into openpmix:master Dec 25, 2025
28 checks passed
@rhc54 rhc54 deleted the topic/fix-alert-11 branch December 25, 2025 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant