Class with Virtual Method without a Virtual Destructor

Description

Class with Virtual Method without a Virtual Destructor occurs when a class that contains one or more virtual methods lacks an associated virtual destructor. In C++, if a class has virtual methods, it is likely to be used polymorphically (through base class pointers). When such objects are deleted through a base pointer without a virtual destructor, only the base destructor is called, leading to resource leaks and undefined behavior. This is a well-known C++ pitfall that violates the principle that classes with virtual methods should also have virtual destructors.

Risk

Missing virtual destructors in classes with virtual methods have direct security implications. Resource leaks from incomplete destruction can cause denial of service. Memory corruption from improper destruction can be exploited. The undefined behavior from partial destruction is unpredictable and potentially exploitable. Security-critical cleanup code in derived classes may not execute, leaving sensitive data in memory or resources unlocked. The reliability issues accumulate over time, eventually causing system failures.

Solution

If a class has any virtual method, always make its destructor virtual. Apply the C++ Core Guidelines: "If a class has any virtual function, it should have a virtual destructor." Use static analysis tools that detect this pattern. Consider using the override keyword in C++11+ to make intentions clear. In base classes intended for inheritance, mark the destructor as virtual even if currently no virtual methods exist. Use smart pointers which help mitigate (but don't fully solve) the problem. Review class hierarchies for missing virtual destructors.

Common Consequences

Impact	Details
Availability	Scope: Availability DoS: Resource Consumption - Memory and resource leaks from incomplete destruction.
Other	Scope: Other Reduce Reliability - Undefined behavior from improper destruction causes unpredictable behavior.
Integrity	Scope: Integrity Unexpected State - Derived class cleanup not executed, leaving inconsistent state.

Example Code

Vulnerable Code

// Vulnerable: Virtual method but non-virtual destructor
class VulnerableShape {
protected:
    double x, y;
    char* name;

public:
    VulnerableShape(const char* n) {
        name = new char[strlen(n) + 1];
        strcpy(name, n);
    }

    // Non-virtual destructor (BUG!)
    ~VulnerableShape() {
        delete[] name;
        std::cout << "Shape destroyed" << std::endl;
    }

    // Virtual method - indicates polymorphic usage
    virtual double area() const = 0;

    virtual void draw() const {
        std::cout << "Drawing shape: " << name << std::endl;
    }
};

class VulnerableCircle : public VulnerableShape {
private:
    double radius;
    double* computedValues;  // Additional resource

public:
    VulnerableCircle(const char* n, double r)
        : VulnerableShape(n), radius(r) {
        computedValues = new double[100];
        precomputeValues();
    }

    ~VulnerableCircle() {
        // This destructor is NEVER called when deleting through base pointer!
        delete[] computedValues;
        std::cout << "Circle destroyed" << std::endl;
    }

    double area() const override {
        return 3.14159 * radius * radius;
    }
};

void vulnerableUsage() {
    // Create circle through base pointer
    VulnerableShape* shape = new VulnerableCircle("MyCircle", 5.0);

    shape->draw();
    std::cout << "Area: " << shape->area() << std::endl;

    // Vulnerable: Only VulnerableShape destructor called!
    delete shape;
    // Output: "Shape destroyed" (only!)
    // Memory leak: computedValues never freed!
}

// Vulnerable: Interface with virtual methods but no virtual destructor
class VulnerableEventHandler {
public:
    // Non-virtual destructor in interface!
    ~VulnerableEventHandler() {}

    virtual void onEvent(const Event& event) = 0;
    virtual void onError(const std::string& error) = 0;
};

class VulnerableLoggingHandler : public VulnerableEventHandler {
private:
    std::ofstream logFile;
    std::mutex logMutex;
    std::vector<std::string> buffer;

public:
    VulnerableLoggingHandler(const std::string& filename) {
        logFile.open(filename, std::ios::app);
    }

    ~VulnerableLoggingHandler() {
        // Never called through base pointer!
        flush();
        logFile.close();
    }

    void onEvent(const Event& event) override {
        std::lock_guard<std::mutex> lock(logMutex);
        buffer.push_back(event.toString());
    }

    void onError(const std::string& error) override {
        std::lock_guard<std::mutex> lock(logMutex);
        buffer.push_back("ERROR: " + error);
        flush();  // Flush on errors
    }

private:
    void flush() {
        for (const auto& entry : buffer) {
            logFile << entry << std::endl;
        }
        buffer.clear();
    }
};

void vulnerableHandlerUsage() {
    std::vector<VulnerableEventHandler*> handlers;

    handlers.push_back(new VulnerableLoggingHandler("app.log"));

    // Process events...
    Event event;
    for (auto& handler : handlers) {
        handler->onEvent(event);
    }

    // Cleanup - destructors not properly called!
    for (auto& handler : handlers) {
        delete handler;
        // Log file never flushed, never closed!
        // Buffered log entries lost!
    }
}

Fixed Code

// Fixed: Virtual destructor with virtual methods
class FixedShape {
protected:
    double x, y;
    std::string name;  // Use std::string instead of raw pointer

public:
    FixedShape(const std::string& n) : name(n) {}

    // Fixed: Virtual destructor
    virtual ~FixedShape() {
        std::cout << "Shape destroyed: " << name << std::endl;
    }

    // Virtual method
    virtual double area() const = 0;

    virtual void draw() const {
        std::cout << "Drawing shape: " << name << std::endl;
    }
};

class FixedCircle : public FixedShape {
private:
    double radius;
    std::unique_ptr<double[]> computedValues;  // Smart pointer

public:
    FixedCircle(const std::string& n, double r)
        : FixedShape(n), radius(r),
          computedValues(std::make_unique<double[]>(100)) {
        precomputeValues();
    }

    // Override destructor - will be called properly
    ~FixedCircle() override {
        std::cout << "Circle destroyed: " << getName() << std::endl;
        // computedValues automatically cleaned up by unique_ptr
    }

    double area() const override {
        return 3.14159 * radius * radius;
    }

private:
    void precomputeValues() {
        // Precompute values...
    }
};

void fixedUsage() {
    // Fixed: Proper destruction through base pointer
    FixedShape* shape = new FixedCircle("MyCircle", 5.0);

    shape->draw();
    std::cout << "Area: " << shape->area() << std::endl;

    // Fixed: Both destructors called in correct order
    delete shape;
    // Output: "Circle destroyed: MyCircle"
    //         "Shape destroyed: MyCircle"
}

// Even better: Use smart pointers
void modernUsage() {
    std::unique_ptr<FixedShape> shape =
        std::make_unique<FixedCircle>("MyCircle", 5.0);

    shape->draw();
    // Automatic cleanup when out of scope
}

// Fixed: Interface with pure virtual destructor
class FixedEventHandler {
public:
    // Fixed: Pure virtual destructor with definition
    virtual ~FixedEventHandler() = 0;

    virtual void onEvent(const Event& event) = 0;
    virtual void onError(const std::string& error) = 0;
};

// Must provide definition for pure virtual destructor
FixedEventHandler::~FixedEventHandler() = default;

class FixedLoggingHandler final : public FixedEventHandler {
private:
    std::ofstream logFile;
    std::mutex logMutex;
    std::vector<std::string> buffer;

public:
    FixedLoggingHandler(const std::string& filename) {
        logFile.open(filename, std::ios::app);
        if (!logFile) {
            throw std::runtime_error("Cannot open log file");
        }
    }

    ~FixedLoggingHandler() override {
        // Fixed: Will be called properly
        flush();
        logFile.close();
    }

    void onEvent(const Event& event) override {
        std::lock_guard<std::mutex> lock(logMutex);
        buffer.push_back(event.toString());

        if (buffer.size() >= 100) {
            flush();
        }
    }

    void onError(const std::string& error) override {
        std::lock_guard<std::mutex> lock(logMutex);
        buffer.push_back("ERROR: " + error);
        flush();
    }

private:
    void flush() {
        for (const auto& entry : buffer) {
            logFile << entry << std::endl;
        }
        buffer.clear();
        logFile.flush();
    }
};

void fixedHandlerUsage() {
    // Fixed: Use smart pointers for automatic cleanup
    std::vector<std::unique_ptr<FixedEventHandler>> handlers;

    handlers.push_back(std::make_unique<FixedLoggingHandler>("app.log"));

    // Process events...
    Event event;
    for (auto& handler : handlers) {
        handler->onEvent(event);
    }

    // Fixed: Automatic cleanup - all destructors called properly
    handlers.clear();
    // Or just let them go out of scope
}

// C++ Core Guidelines compliant abstract base class
class AbstractBase {
public:
    // Rule: A polymorphic class should suppress copying
    AbstractBase() = default;
    AbstractBase(const AbstractBase&) = delete;
    AbstractBase& operator=(const AbstractBase&) = delete;

    // Rule: Virtual destructor for polymorphic class
    virtual ~AbstractBase() = default;

    // Pure virtual method
    virtual void doSomething() = 0;
};

// Alternative: Protected non-virtual destructor for non-polymorphic deletion
class NonPolymorphicBase {
public:
    virtual void doSomething() = 0;

protected:
    // Non-virtual but protected - can't delete through base pointer
    ~NonPolymorphicBase() = default;
};

// This prevents dangerous usage:
// NonPolymorphicBase* ptr = new Derived();
// delete ptr;  // Compile error! Destructor is protected

---

CVE Examples

Memory leaks and resource leaks from missing virtual destructors have contributed to denial-of-service vulnerabilities, though specific CVEs typically describe the impact rather than this specific cause.

---

Related CWEs

• CWE-1076: Insufficient Adherence to Expected Conventions (parent)
• CWE-1079: Parent Class without Virtual Destructor Method (closely related)
• CWE-401: Missing Release of Memory after Effective Lifetime (can result from)

---

References

1. MITRE Corporation. "CWE-1087: Class with Virtual Method without a Virtual Destructor." https://cwe.mitre.org/data/definitions/1087.html
2. C++ Core Guidelines. C.35: A base class destructor should be either public and virtual, or protected and non-virtual.
3. Meyers, Scott. "Effective C++." Item 7: Declare destructors virtual in polymorphic base classes.