Bug report] BinFHEContext::SwitchCTtoqn accepts malformed LWESwitchingKey tensors and crashes in KeySwitch

Hi OpenFHE team,

I would like to report an input-validation issue in the BinFHE key-switching path that can lead to unchecked out-of-bounds access in KeySwitch(), and ask whether malformed LWESwitchingKey shape is expected to be validated anywhere before it is used.

Summary

LWEEncryptionScheme::KeySwitch() assumes that the nested LWESwitchingKey tensors match the expected N, baseKS, digitCount, and n dimensions.

The public SwitchCTtoqn() path accepts a non-null but structurally malformed switching key and forwards it into that indexing logic.

In the reproducer below, the inner tensor structure is preserved from a valid switching key, but the outer A / B dimensions are truncated from N to 1. The ciphertext argument is kept shape-compatible with the large-dimension SwitchCTtoqn() path. That is enough to reach unchecked indexing in KeySwitch() and crash with an ASan heap-buffer-overflow.

I reproduced this locally under ASan/UBSan on OpenFHE 1.5.1.

Separately, source inspection suggests that deserialization and BTKeyLoad() also do not perform obvious shape validation, but the current reproducer does not go through those paths.

Environment

  • OpenFHE version: 1.5.1
  • Compiler: clang 14.0.0
  • Sanitizers: AddressSanitizer, UndefinedBehaviorSanitizer

Minimal reproduction

The reproducer below starts from a valid switching key, preserves the first outer element exactly, truncates the outer A / B dimensions to size 1, and then calls SwitchCTtoqn() with a shape-compatible large-dimension ciphertext:

#include "binfhecontext.h"
#include "lwe-keyswitchkey.h"

using namespace lbcrypto;

int main() {
    BinFHEContext cc;
    cc.GenerateBinFHEContext(TOY);

    auto sk = cc.KeyGen();
    cc.BTKeyGen(sk);
    auto valid = cc.GetSwitchKey();
    const auto& lweParams = cc.GetParams()->GetLWEParams();
    NativeVector a(lweParams->GetN(), lweParams->GetQ());
    auto ctLarge = std::make_shared<LWECiphertextImpl>(std::move(a), 0);

    using KeyA = std::vector<std::vector<std::vector<NativeVector>>>;
    using KeyB = std::vector<std::vector<std::vector<NativeInteger>>>;

    KeyA shortKeyA = { valid->GetElementsA()[0] };
    KeyB shortKeyB = { valid->GetElementsB()[0] };

    auto malformed =
        std::make_shared<LWESwitchingKeyImpl>(std::move(shortKeyA), std::move(shortKeyB));

    auto out = cc.SwitchCTtoqn(malformed, ctLarge);
    (void)out;

    return 0;
}

Expected behavior

The malformed switching key should be rejected before KeySwitch() starts indexing its nested vectors.

Actual behavior

The process crashes with an ASan heap-buffer-overflow in KeySwitch().

Relevant excerpt:

ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 8 at ...
    #0  ... std::vector<std::vector<intnat::NativeIntegerT<unsigned long>, ...>>::operator[](...)
        .../stl_vector.h:1143:25
    #1  ... lbcrypto::LWEEncryptionScheme::KeySwitch(...) .../lwe-pke.cpp:340:28
    #2  ... lbcrypto::LWEEncryptionScheme::SwitchCTtoqn(...) .../lwe-pke.cpp:158:17
    #3  ... lbcrypto::BinFHEContext::SwitchCTtoqn(...) .../binfhecontext.cpp:260:25

The sanitizer line numbers above refer to my local instrumented source tree; stable v1.5.1 source links are listed below.

In this outer-dimension-truncation reproducer, once the loop reaches i == 1, the following outer indexing operations are out of bounds because both key tensors have size 1:

auto& refA = K->GetElementsA()[i];
auto& refB = K->GetElementsB()[i];

In the run above, the first sanitizer-observed invalid read occurs later at:

b.ModSubFastEq(refB[a0][j], Q);

after refB has already been derived from an out-of-bounds outer element.

Cause analysis

The public SwitchCTtoqn() path accepts a non-null but structurally malformed LWESwitchingKey and forwards it to KeySwitch().

The direct root cause is that KeySwitch() derives N, n, baseKS, and digitCount from LWECryptoParams, then indexes the key tensors as though every corresponding dimension were present:

auto& refA = K->GetElementsA()[i];
auto& refB = K->GetElementsB()[i];
...
b.ModSubFastEq(refB[a0][j], Q);
auto& refAj = refA[a0][j];
for (uint32_t k = 0; k < n; ++k)
    a[k].ModSubFastEq(refAj[k], Q);

The canonical tensor shape is also directly visible in KeySwitchGen(), which generates:

  • outer A dimension == N and outer B dimension == N
  • second dimension == baseKS
  • third dimension == digitCount
  • each NativeVector length == n

For bounds safety, KeySwitch() requires at least:

  • keyA.size() >= N and keyB.size() >= N
  • for every i, keyA[i].size() >= baseKS and keyB[i].size() >= baseKS
  • for every i, a0, keyA[i][a0].size() >= digitCount and keyB[i][a0].size() >= digitCount
  • for every i, a0, j, keyA[i][a0][j].GetLength() >= n

Separately, source inspection suggests additional trust boundaries:

  • LWESwitchingKeyImpl::load() deserializes m_keyA and m_keyB without structural validation
  • BTKeyLoad() assigns the supplied bootstrapping-key structure without checking its compatibility with the current context parameters

Those observations suggest additional entry paths, but the current reproducer does not dynamically prove them.

Relevant source locations

Suggested direction

It looks reasonable to validate the switching-key shape at the beginning of KeySwitch(), because that function has direct access to the active LWECryptoParams and is the authority that consumes the tensor.

An optional matching early check in BTKeyLoad() would also make sense because BinFHEContext already holds m_params.

By contrast, LWESwitchingKeyImpl::load() can only validate self-consistency of the serialized tensors unless the expected parameter set is also provided.

For bounds safety, KeySwitch() should verify at least:

  • keyA.size() >= N and keyB.size() >= N
  • for every i, keyA[i].size() >= baseKS and keyB[i].size() >= baseKS
  • for every i, a0, keyA[i][a0].size() >= digitCount and keyB[i][a0].size() >= digitCount
  • for every i, a0, j, keyA[i][a0][j].GetLength() >= n

If switching keys are required to have the canonical shape produced by KeySwitchGen(), the implementation could instead enforce exact equality:

  • keyA.size() == N and keyB.size() == N
  • for every i, keyA[i].size() == baseKS and keyB[i].size() == baseKS
  • for every i, a0, keyA[i][a0].size() == digitCount and keyB[i][a0].size() == digitCount
  • for every i, a0, j, keyA[i][a0][j].GetLength() == n

Question

Is malformed LWESwitchingKey shape supposed to be rejected explicitly, or is this currently treated as an unchecked internal precondition?

Reported by Jiang Chao, Beijing University of Posts and Telecommunications

Just like with other similar issues, our focus is on the honest-but-curios threat model.