[Bug report] "Discrete Gaussian Generator Generic" shallow-copies owned SamplerCombiner pointers, causing use-after-free on destruction

Library Questions
1

Hi OpenFHE team,

I would like to report a reproducible C++ lifetime bug in the public DiscreteGaussianGeneratorGeneric class.

Summary

DiscreteGaussianGeneratorGeneric allocates SamplerCombiner objects with new, stores them in raw pointer members, and releases them in its destructor. However, the class does not delete or define its copy special members, so it remains implicitly copy-constructible and copy-assignable.

As a result, an ordinary copy such as:

DiscreteGaussianGeneratorGeneric duplicate = original;

shallow-copies the owned combiners pointers. When both objects later leave scope, the second destructor operates on already-freed SamplerCombiner objects. On my ASan/UBSan build this produces an invalid-vptr diagnostic and a heap-use-after-free in ~DiscreteGaussianGeneratorGeneric().

Source inspection also shows two closely related issues from the same raw-ownership design:

  • copy assignment would first orphan the destination’s existing combiners and then create the same duplicated-ownership / double-destruction problem;
  • constructor failure is not exception-safe: if a later new SamplerCombiner(...) throws, earlier allocations leak because the partially constructed object is never destroyed.

Environment

  • OpenFHE tested revision: ed361af22049007db2107e7c69bcff209e8c420d
  • OS: Linux x86_64
  • Compiler: Clang 14.0.0
  • Sanitizers: AddressSanitizer, UndefinedBehaviorSanitizer

Representative OpenFHE build:

cmake -S openfhe-development -B build-openfhe-asan \
  -DCMAKE_C_COMPILER=clang-14 \
  -DCMAKE_CXX_COMPILER=clang++-14 \
  -DCMAKE_BUILD_TYPE=Debug \
  -DBUILD_STATIC=ON \
  -DBUILD_SHARED=OFF \
  -DBUILD_UNITTESTS=OFF \
  -DBUILD_EXAMPLES=OFF \
  -DBUILD_BENCHMARKS=OFF \
  -DWITH_OPENMP=OFF \
  -DCMAKE_CXX_FLAGS='-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1' \
  -DCMAKE_C_FLAGS='-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1'
cmake --build build-openfhe-asan -j

Minimal reproduction

The following program uses only public core headers and types. It constructs a DiscreteGaussianGeneratorGeneric with b = 1, so it supplies the expected two base samplers centered at 0.0 and 0.5. It then copy-constructs a second instance and lets both objects leave scope. The chosen parameters also satisfy the documented base-sampler condition 8.0 >= 4 * sqrt(2) * 1.0.

#include "math/discretegaussiangeneratorgeneric.h"

#include <iostream>
#include <type_traits>

using namespace lbcrypto;

static_assert(std::is_copy_constructible_v<DiscreteGaussianGeneratorGeneric>);
static_assert(std::is_copy_assignable_v<DiscreteGaussianGeneratorGeneric>);

int main() {
    BitGenerator bg;
    BaseSampler base0(0.0, 8.0, &bg, PEIKERT);
    BaseSampler base1(0.5, 8.0, &bg, PEIKERT);
    BaseSampler* samplers[2] = {&base0, &base1};

    std::cout << "constructing original" << std::endl;
    DiscreteGaussianGeneratorGeneric original(samplers, 8.0, 1, 1.0);

    std::cout << "copy-constructing duplicate" << std::endl;
    DiscreteGaussianGeneratorGeneric duplicate = original;

    std::cout << "leaving scope" << std::endl;
    return 0;
}

Representative probe build:

clang++-14 -std=gnu++17 -fno-omit-frame-pointer -g -O1 \
  -fsanitize=address,undefined \
  openfhe_dgg_copy_probe.cpp \
  -Iopenfhe-development/src/core/include \
  -Iopenfhe-development/third-party/include \
  -Iopenfhe-development/third-party/cereal/include \
  -Ibuild-openfhe-asan/src/core \
  build-openfhe-asan/lib/libOPENFHEcore_static.a \
  -lpthread -ldl -lm \
  -o openfhe_dgg_copy_probe

Run:

ASAN_OPTIONS=detect_leaks=0 ./openfhe_dgg_copy_probe

Actual behavior

On my setup, the program printed:

constructing original
copy-constructing duplicate
leaving scope

and then terminated with this representative sanitizer output:

runtime error: member call on address 0x... which does not point to an object of type 'lbcrypto::BaseSampler'
...
ERROR: AddressSanitizer: heap-use-after-free on address 0x...
READ of size 8 at 0x... thread T0
#0 lbcrypto::DiscreteGaussianGeneratorGeneric::~DiscreteGaussianGeneratorGeneric()
   at src/core/lib/math/discretegaussiangeneratorgeneric.cpp:306
...
freed by thread T0 here:
#1 lbcrypto::SamplerCombiner::~SamplerCombiner()
   at src/core/include/math/discretegaussiangeneratorgeneric.h:287
#2 lbcrypto::DiscreteGaussianGeneratorGeneric::~DiscreteGaussianGeneratorGeneric()
   at src/core/lib/math/discretegaussiangeneratorgeneric.cpp:306
...
previously allocated by thread T0 here:
#1 lbcrypto::DiscreteGaussianGeneratorGeneric::DiscreteGaussianGeneratorGeneric(...)
   at src/core/lib/math/discretegaussiangeneratorgeneric.cpp:285

So a normal copy of the public class produces duplicated ownership of the same heap-allocated combiners, and the second destructor then operates on already freed objects.

Expected behavior

If DiscreteGaussianGeneratorGeneric owns its internal combiner chain, a normal copy should not create aliased ownership of those heap allocations.

The class should therefore do one of the following explicitly:

  • be non-copyable; or
  • provide a deep-copy implementation; or
  • use move-only RAII ownership and transfer that ownership safely.

In all cases, destroying two instances must not operate on the same freed SamplerCombiner objects.

Constructor failure should also not leak already-allocated combiners.

Cause analysis

The relevant public class definition is:

class DiscreteGaussianGeneratorGeneric {
public:
    DiscreteGaussianGeneratorGeneric(BaseSampler** samplers,
                                     const double std,
                                     const int b,
                                     double N);
    ~DiscreteGaussianGeneratorGeneric();

private:
    BaseSampler* wide_sampler;
    BaseSampler** base_samplers;
    BaseSampler* combiners[MAX_LEVELS];
    ...
};

The constructor allocates owned combiners one by one:

wide_sampler  = samplers[0];
wide_variance = base_variance;
for (int i = 1; i < MAX_LEVELS; ++i) {
    ...
    wide_sampler     = new SamplerCombiner(wide_sampler, wide_sampler, x1, x2);
    combiners[i - 1] = wide_sampler;
    ...
}

and the destructor later deletes them:

for (int i = 1; i < MAX_LEVELS; ++i) {
    delete combiners[i - 1];
}

Because the class declares a destructor but does not delete or define copy construction / copy assignment, the compiler still generates implicit copy operations. Those copy operations shallow-copy:

  • wide_sampler
  • base_samplers
  • every entry in combiners

The dynamic repro above shows the concrete consequence: both objects believe they own the same SamplerCombiner allocations, so the second destructor reads through freed memory and then attempts to destroy the same objects again.

More generally, the problem is not limited to the final destructor call. Once either copied instance destroys the shared combiner chain, the other instance’s wide_sampler and combiners[] members become dangling pointers. A later call to the two-argument overload, such as GenerateInteger(0.0, 1000000.0), on the still-live copy would then dereference the dangling wide_sampler.

Copy assignment is also problematic even before destruction. It would overwrite the destination’s existing combiners pointers without freeing them first, leaking that combiner chain, and would then leave source and destination sharing the same newly copied pointers.

The constructor is also not exception-safe. If one of the later new SamplerCombiner(...) calls throws, any combiners allocated in earlier iterations remain leaked because the partially constructed DiscreteGaussianGeneratorGeneric object will never reach its destructor.
That exception-safety issue is source-confirmed from the same allocation pattern but was not separately exercised by the primary reproducer above.

Impact

The confirmed impact is a sanitizer-detected use-after-free during destruction after an ordinary copy construction of a public C++ class.

This is a native C++ ownership / lifetime bug, not a malformed-input parsing issue. A caller that copies DiscreteGaussianGeneratorGeneric by value can reach it without violating the class’s visible type contract, because the class currently appears copyable.

The constructor leak is a source-confirmed additional issue from the same raw ownership design.

Relevant source locations

Suggested direction

The owned combiner chain should be expressed with RAII rather than raw owning pointers. For example:

std::array<std::unique_ptr<SamplerCombiner>, MAX_LEVELS - 1> combiners;

If the class is not intended to be copyable, the simplest safe change would be to delete copy construction and copy assignment explicitly:

DiscreteGaussianGeneratorGeneric(const DiscreteGaussianGeneratorGeneric&) = delete;
DiscreteGaussianGeneratorGeneric& operator=(const DiscreteGaussianGeneratorGeneric&) = delete;

If move support is desired, it can then be implemented explicitly on top of move-only ownership. The move operations should preserve the internal combiner links and keep non-owning observer members such as wide_sampler coherent.
Depending on the final member layout, the move special members may then be explicitly defaultable.

The constructor should also build its combiner chain in exception-safe RAII state so that partial allocation failures do not leak.

Reported by Jiang Chao, Beijing University of Posts and Telecommunications