BinFHEContext::Decrypt accepts undersized ciphertexts, leading to an out-of-bounds read

Bug Reports
1

Hi OpenFHE team,

I would like to report an input-validation issue in the BinFHE decryption path that can lead to an out-of-bounds read, and ask whether malformed ciphertext length is supposed to be validated at this API boundary.

Summary

BinFHEContext::Decrypt() forwards a malformed LWECiphertext into LWEEncryptionScheme::Decrypt() without verifying that the ciphertext length matches the secret-key length.

The lower-level decryption loop uses the secret-key length as its loop bound while indexing the ciphertext vector.

With the TOY parameter set, replacing a valid ciphertext’s a vector with a one-element vector is sufficient to trigger an ASan heap-buffer-overflow in the decryption loop.

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

Environment

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

Minimal reproduction

The reproducer below starts from a valid ciphertext and then replaces its a vector with a one-element vector:

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

using namespace lbcrypto;

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

    auto sk = cc.KeyGen();
    auto ct = cc.Encrypt(sk, 1);

    NativeVector shortA(1, ct->GetModulus());
    shortA[0] = ct->GetA()[0];

    auto malformed =
        std::make_shared<LWECiphertextImpl>(std::move(shortA), ct->GetB(), ct->GetptModulus());

    LWEPlaintext result = 0;
    cc.Decrypt(sk, malformed, &result);

    return 0;
}

Expected behavior

Decrypt() should reject the input if ct->GetLength() != sk->GetLength() before indexing ct->GetA().

Actual behavior

The process crashes with an ASan heap-buffer-overflow caused by an out-of-bounds read from the ciphertext’s a vector.

Relevant excerpt:

==...==ERROR: AddressSanitizer: heap-buffer-overflow
    #0  ... NativeIntegerT<unsigned long>::ModMulFast(...) .../ubintnat.h:1353
    #1  ... lbcrypto::LWEEncryptionScheme::Decrypt(...) .../lwe-pke.cpp:189
    #2  ... lbcrypto::BinFHEContext::Decrypt(...) .../binfhecontext.cpp:269

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

Root cause analysis

BinFHEContext::Decrypt() checks only whether sk and ct are null and then forwards the arguments. Neither the context-level function nor the lower-level decryption path validates that the ciphertext and secret key have matching lengths.

Inside LWEEncryptionScheme::Decrypt(), the loop bound is taken from the secret-key length:

const auto& a = ct->GetA();
auto s        = sk->GetElement();
uint32_t n    = s.GetLength();
...
for (uint32_t i = 0; i < n; ++i) {
    inner += a[i].ModMulFast(s[i], q, mu);
}

The implementation already contains a TODO about checking secret-key and ciphertext parameter compatibility, but no dimension check is currently performed:

// TODO in the future we should add a check to make sure sk parameters match
// the ct parameters

The loop bound n is derived from s.GetLength(), while the indexed vector a has length ct->GetLength(). No invariant connects these two lengths before the loop.

Therefore, whenever ct->GetLength() < sk->GetLength(), the loop eventually evaluates a[i] with i >= a.GetLength().

Relevant source locations

Suggested direction

It looks reasonable to check ct->GetLength() == sk->GetLength() before entering the decryption loop.

I think the most complete place for that check would be inside LWEEncryptionScheme::Decrypt() before the loop, with an optional matching early check in BinFHEContext::Decrypt().

Question

Is BinFHEContext::Decrypt() expected to reject malformed ciphertext dimensions, or is that currently considered a caller-side precondition only?

Reported by Jiang Chao, Beijing University of Posts and Telecommunications

2

The answer is the same as for other similar issues. Our main focus is on the honest-but-curios thread model.