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
Adimension== Nand outerBdimension== N - second dimension
== baseKS - third dimension
== digitCount - each
NativeVectorlength== n
For bounds safety, KeySwitch() requires at least:
keyA.size() >= NandkeyB.size() >= N- for every
i,keyA[i].size() >= baseKSandkeyB[i].size() >= baseKS - for every
i, a0,keyA[i][a0].size() >= digitCountandkeyB[i][a0].size() >= digitCount - for every
i, a0, j,keyA[i][a0][j].GetLength() >= n
Separately, source inspection suggests additional trust boundaries:
LWESwitchingKeyImpl::load()deserializesm_keyAandm_keyBwithout structural validationBTKeyLoad()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
src/binfhe/lib/binfhecontext.cppBinFHEContext::SwitchCTtoqn(): lines 228-237- GitHub link: https://github.com/openfheorg/openfhe-development/blob/v1.5.1/src/binfhe/lib/binfhecontext.cpp#L228-L237
src/binfhe/include/lwe-keyswitchkey.hLWESwitchingKeyImpltensor constructors: lines 50-55LWESwitchingKeyImpl::load(): lines 107-115- GitHub links:
src/binfhe/include/binfhecontext.hBTKeyLoad(): lines 254-261- assignment of the supplied key material: lines 259-260
- GitHub link: https://github.com/openfheorg/openfhe-development/blob/v1.5.1/src/binfhe/include/binfhecontext.h#L254-L261
src/binfhe/lib/lwe-pke.cppLWEEncryptionScheme::KeySwitchGen(): lines 226-288LWEEncryptionScheme::KeySwitch(): lines 292-315- nested accesses: lines 302-311
- GitHub links:
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() >= NandkeyB.size() >= N- for every
i,keyA[i].size() >= baseKSandkeyB[i].size() >= baseKS - for every
i, a0,keyA[i][a0].size() >= digitCountandkeyB[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() == NandkeyB.size() == N- for every
i,keyA[i].size() == baseKSandkeyB[i].size() == baseKS - for every
i, a0,keyA[i][a0].size() == digitCountandkeyB[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