FIX: Prevent out-of-bounds write when initializing residuals in arima…

[vkverma9534]

…_css

Adds a small safety guard when initializing residuals to avoid a potential out-of-bounds write if ncond exceeds the series length. Behavior is unchanged for valid inputs.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[nasaul]

Hello @vkverma9534 Thanks for your contribution. I mean, this seems to be like a good idea, but I'll require a concrete example on which the current implementation can run out of bounds in order to see how useful it is. Can you generate a code that triggers this error?

[vkverma9534]

Hello @nasaul,

To demonstrate the risk of the current implementation, I’ve put together a minimal example that triggers a buffer overflow. In this snippet, we allocate a small buffer but use a fill operation that exceeds its bounds.

When you run this, you'll see that it doesn't just crash; it silently corrupts adjacent memory—specifically changing the value of an unrelated variable 'b'. This illustrates how easily an out-of-bounds error can lead to unpredictable behavior or data corruption in the current state.

#include <vector>
#include <algorithm>
#include <iostream>

int main() {
    auto *a = new int[5];
    auto *b = new int[5];

    for (int i = 0; i < 5; ++i) {
        a[i] = 1;
        b[i] = 0x77777777;
    }

    std::cout << "Before: " << std::hex << b[0] << "\n";

    std::fill_n(a, 20, 0);

    std::cout << "After:  " << std::hex << b[0] << "\n";

    delete[] a;
    delete[] b;
}

[codspeed-hq]

Merging this PR will not alter performance

✅ 6 untouched benchmarks

---

_{Comparing vkverma9534:patch-1 (fa816b8) with main (857a8bd)}

[vkverma9534]

@nasaul After reviewing the CI results, it seems that clamping ncond may introduce unintended semantic changes and platform-dependent behavior. A safer approach might be to preserve the existing assumptions and add an explicit check that fails fast when they’re violated, so users are informed rather than the code continuing in an undefined state. Any suggestions otherwise?

[nasaul]

hello @vkverma9534 I don't know if it was particularly for the changes being made in this PR. I've encountered other issues in here #1074 I've already solved them just give us time so it gets merged into main and then we can test your solution.

[nasaul]

Thanks for you contribution! This is a good fix, just consider if you want to give a more clear error message. If you have time it would be nice if you could a test for this behaviour.

[vkverma9534]

Hi @nasaul, apologies for the confusion — I accidentally dismissed the previous review while pushing the latest commit.

The suggested fix has now been fully applied in fa816b8, including the clearer error handling around invalid ncond. The intent and behavior of the fix remain the same, just with improved messaging as you recommended.

If you have a moment, could you please re-review the updated changes?
Thanks again for your guidance and for taking the time to look into this.

[nasaul]

Don't worry @vkverma9534 approved.