RotationKey error

Library Questions
1

Hi OpenFHE Team!

While running the below code with the UBSAN tool, a potential OpenFHE-related error was detected.

cc->EvalAtIndexKeyGen(keys.secretKey, {1, -2});
cc->EvalRotateKeyGen(keys.secretKey, {1, -2});

In the two lines of code responsible for generating keys(Each of these lines individually triggers a UBSAN message, and I believe both lines lead to a call to this function, which is why I included them together), setting the values to -2, -3, or further negative numbers causes UBSAN to issue an overflow error. which is /data/homomorphic/openfhe-1.2.2/openfhe-development/src/core/lib/math/nbtheory2.cpp:269:20: runtime error: signed integer overflow: 52429 * 52429 cannot be represented in type 'int'. This led us to further examine line 269 of the file nbtheory2.cpp for a deeper understanding.

uint32_t FindAutomorphismIndex2nComplex(int32_t i, uint32_t m) {
    if (i == 0) {
        return 1;
    }

    // conjugation automorphism
    if (i == int32_t(m - 1)) {
        return uint32_t(i);
    }
    else {
        // generator
        int32_t g0;

        if (i < 0) {
            g0 = NativeInteger(5).ModInverse(m).ConvertToInt();
        }
        else {
            g0 = 5;
        }
        uint32_t i_unsigned = (uint32_t)std::abs(i);

        int32_t g = g0;
        for (size_t j = 1; j < i_unsigned; j++) {
            g = (g * g0) % m;
        }
        return uint32_t(g);
    }
}

We are Examining the source code of the error message reveals g and g0 variable are int32_t , but eventually return uint32_t. It will cause the potential signed integer overflow.
Additionally, while searching through other code files in the library, we found that this variable is used with the uint32_t type in other parts of the code, which prevents overflow issues.
Here is the code——NativeInteger(5).ModInverse(m).ConvertToInt<uint32_t>();

Is this a bug?

Thank you for taking the time to look into this issue. Please let us know if any further details or clarification would be helpful!

Best regards,
wowblk

2

From an initial analysis, there seems to be a semantic bug that does not affect the functionality (the final modular result is correct), but only because the cyclotomic order is a power of 2 and the underlying number representation system in the processor is binary.
We will investigate this further. To address this issue, a Github issue has been created.

Thank you for reporting this.