Skip to content

[Bug report] Evaluator::add_many leaves destination modified after throwing on a later ciphertext parameter mismatch #760

Description

@CCYJ1014

Hi SEAL team,
I would like to report a reproducible failed-call state mutation / exception-safety issue in SEAL's public C++ API. I originally found this during SEAL v4.4.0 testing; I reran the reproducer against the latest SEAL v4.4.3 release, and the same failed-call state mutation is still present.

Evaluator::add_many assigns:

destination = encrypteds[0];

before checking later ciphertexts for the compatibility conditions enforced by add_inplace. If a later ciphertext has a different parms_id, the function throws from add_inplace, but the caller's destination has already been overwritten with the first input ciphertext. This report is therefore about failed-call output state / exception-safety semantics, not a validation bypass or a memory-safety issue.

Summary

I reproduced this on SEAL v4.4.3:

  • tested revision: 356f2e6dcc0520dc9fc14e98674d9a56cba6018c

The reproducer uses only valid public objects:

  1. create a valid BFV context
  2. encrypt 11 as the first ciphertext
  3. create a second ciphertext by modulus-switching that valid ciphertext once
  4. initialize destination to an unrelated valid sentinel ciphertext encoding 99, then modulus-switch it once so its parms_id differs from first
  5. call Evaluator::add_many({first, mismatched}, destination)

The call throws:

encrypted1 and encrypted2 parameter mismatch

but destination is no longer the sentinel. After the exception, it decrypts to 11, and its parms_id now matches first, i.e. both payload and metadata have already been overwritten with encrypteds[0].

This is separate from the one-element add_many validation-bypass issue. In this reproducer, add_many does report the documented parameter mismatch; the question is the state left in the caller-provided output object after that failed call.

Environment

  • SEAL release/tag: v4.4.3
  • Tested revision: 356f2e6dcc0520dc9fc14e98674d9a56cba6018c
  • OS: Linux x86_64
  • Compiler: Clang 14.0.0
  • Build: local debug build for path confirmation

Original discovery:

  • SEAL release/tag: v4.4.0
  • Tested revision: 04d53b99ce745efc26bb4965be609b9894755227

Minimal reproduction

#include <seal/seal.h>

#include <cstdint>
#include <iostream>
#include <vector>

using namespace seal;

static Ciphertext EncryptScalar(
    Encryptor &encryptor, BatchEncoder &encoder, std::uint64_t value)
{
    std::vector<std::uint64_t> slots(encoder.slot_count(), 0ULL);
    slots[0] = value;
    Plaintext plain;
    encoder.encode(slots, plain);
    Ciphertext encrypted;
    encryptor.encrypt(plain, encrypted);
    return encrypted;
}

static std::uint64_t DecryptScalar(
    Decryptor &decryptor, BatchEncoder &encoder, const Ciphertext &encrypted)
{
    Plaintext plain;
    decryptor.decrypt(encrypted, plain);
    std::vector<std::uint64_t> slots;
    encoder.decode(plain, slots);
    return slots.at(0);
}

int main()
{
    EncryptionParameters parms(scheme_type::bfv);
    parms.set_poly_modulus_degree(8192);
    parms.set_coeff_modulus(CoeffModulus::Create(8192, { 50, 50, 50 }));
    parms.set_plain_modulus(PlainModulus::Batching(8192, 20));

    SEALContext context(parms);
    KeyGenerator keygen(context);
    SecretKey sk = keygen.secret_key();
    PublicKey pk;
    keygen.create_public_key(pk);

    Encryptor encryptor(context, pk);
    Evaluator evaluator(context);
    Decryptor decryptor(context, sk);
    BatchEncoder encoder(context);

    Ciphertext first = EncryptScalar(encryptor, encoder, 11);
    Ciphertext mismatched = first;
    evaluator.mod_switch_to_next_inplace(mismatched);

    Ciphertext destination = EncryptScalar(encryptor, encoder, 99);
    evaluator.mod_switch_to_next_inplace(destination);

    std::cout << "before_value="
              << DecryptScalar(decryptor, encoder, destination) << std::endl;
    std::cout << "before_parms_id_eq_first="
              << (destination.parms_id() == first.parms_id()) << std::endl;
    std::cout << "mismatched_parms_id_eq_first="
              << (mismatched.parms_id() == first.parms_id()) << std::endl;

    try
    {
        evaluator.add_many(std::vector<Ciphertext>{ first, mismatched }, destination);
        std::cout << "unexpected_success" << std::endl;
        return 2;
    }
    catch (const std::exception &e)
    {
        std::cout << "caught=" << e.what() << std::endl;
    }

    std::cout << "after_value="
              << DecryptScalar(decryptor, encoder, destination) << std::endl;
    std::cout << "after_parms_id_eq_first="
              << (destination.parms_id() == first.parms_id()) << std::endl;

    const bool mutated_to_first =
        (DecryptScalar(decryptor, encoder, destination) == 11) &&
        (destination.parms_id() == first.parms_id());

    std::cout << "mutated_to_first=" << mutated_to_first << std::endl;
    return mutated_to_first ? 0 : 1;
}

Representative build steps:

cmake -S fuzz/seal-add-many-state-probe \
  -B fuzz/seal-add-many-state-probe/build

cmake --build fuzz/seal-add-many-state-probe/build -j

./fuzz/seal-add-many-state-probe/build/seal_add_many_state_probe

Actual behavior

The program prints:

before_value=99
before_parms_id_eq_first=0
mismatched_parms_id_eq_first=0
caught=encrypted1 and encrypted2 parameter mismatch
after_value=11
after_parms_id_eq_first=1
mutated_to_first=1

So the API reports failure, but destination no longer contains the caller's original sentinel value or its original metadata.

Expected behavior

The current API documentation specifies that incompatible ciphertexts result in an exception, but it does not document that destination may already have been overwritten when that exception is reported.

If this failed-call output mutation is intended, it should be documented explicitly. Otherwise, add_many should avoid mutating destination before the later compatibility checks that can still throw.

Cause analysis

In Evaluator::add_many:

destination = encrypteds[0];
for (size_t i = 1; i < encrypteds.size(); i++)
{
    add_inplace(destination, encrypteds[i]);
}

Compatibility checks such as parms_id equality happen inside add_inplace(...):

if (encrypted1.parms_id() != encrypted2.parms_id())
{
    throw invalid_argument("encrypted1 and encrypted2 parameter mismatch");
}

So the control flow is:

overwrite destination with encrypteds[0]
  -> validate encrypteds[1] only when add_inplace is entered
  -> throw on mismatch
  -> return to caller with destination already changed

I am not claiming that SEAL explicitly documents a strong exception guarantee for destination here. The issue is that callers who catch the documented invalid_argument cannot infer from the API documentation that the previous value of the output object may already have been replaced even though the requested addition did not complete successfully.

This destination-first wrapper shape is not unique to add_many. The destination-taking add, sub, and multiply wrappers also copy the first input into destination before calling the corresponding inplace operation.
So the most conservative interpretation is that this is an API exception-safety / documentation issue rather than an isolated functional correctness bug in add_many.

Impact

This is a failed-call state mutation / exception-state issue

The trigger condition is realistic in normal application code: callers can end up collecting individually valid ciphertexts from different points in the modulus chain and only discover the mismatch when trying to aggregate them.

The direct impact is:

  • the original destination value and metadata are lost even though the API reports failure
  • downstream code that catches the exception may continue with a mutated output object that looks valid but no longer contains the caller's pre-call state

In the reproducer, the post-exception destination is not empty or obviously invalid. It is a structurally valid ciphertext for the current context and decrypts to 11, so subsequent operations may not immediately detect that a failed call replaced the caller's previous output state.

Relevant source locations

Suggested direction

If SEAL intends to preserve the caller's previous output value when a destination-taking operation throws, add_many can compute into a temporary ciphertext and only commit to destination after all additions succeed.

For example, one safe shape would be:

  1. create a temporary result from encrypteds[0]
  2. apply the existing add_inplace loop to the temporary result
  3. assign the temporary to destination only after success

That would avoid the current "exception reported, but destination already overwritten" behavior without requiring a separate full-vector prevalidation pass.

If the current failed-call mutation is intended behavior, then documenting the post-exception state of destination-taking overloads would also address the API ambiguity.

Reported by Jiang Chao, Beijing University of Posts and Telecommunications

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions