hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.46k stars 238 forks source link

[SUGGESTION] Detect UB caused by deleting object via pointer to its base class type with a non-virtual destructor #960

Open bluetarpmedia opened 8 months ago

bluetarpmedia commented 8 months ago

Can Cppfront detect bugs where an object of derived class is deleted via a pointer to its base class that has a non-virtual destructor?

Here's some Cpp2 code demonstrating the problem:

// Deliberately bad code!

Shape: type = {                  // <--- No virtual dtor
    public x: int = ();
    public y: int = ();
}

Circle: type = {
    this: Shape = ();            // <--- Circle inherits from Shape
    radius: float;
    data: std::vector<u8> = ();

    operator=: (out this, x: int, y: int, radius_: float) = {
        radius = radius_;
        Shape::x = x;
        Shape::y = y;

        data.reserve(1024ull);   // <--- Allows LeakSanitizer to confirm the memory leak
    }

    draw: (this) = {
        std::cout << "Draw circle x=(x)$ y=(y)$ radius=(radius)$ \n";
    }
}

print_position: (move shape: std::unique_ptr<Shape>) = {
    std::cout << "Shape position x=(shape*.x)$ y=(shape*.y)$ \n";
}

main: () -> int = {

    circle:= unique.new<Circle>(50, 100, 1.25f);
    circle*.draw();

    print_position(circle);      // <-- Last use of `circle` is moved into `print_position`

    // Alternative:
    // print_position(move circle);

    return 0;
}

This results in undefined behaviour. The unique_ptr<Circle> is converted to unique_ptr<Shape> when circle is moved into print_position, and when the unique_ptr destructs it deletes the Shape * and only the Shape destructor is called.

See Godbolt for an example with LeakSanitizer enabled.

SUMMARY: LeakSanitizer: 1024 byte(s) leaked in 1 allocation(s).

Will your feature suggestion eliminate X% of security vulnerabilities of a given kind in current C++ code? The bug causes undefined behaviour but I've only found one CVE which treats this as a vulnerability:

Will your feature suggestion automate or eliminate X% of current C++ guidance literature? Yes, see:

buola commented 7 months ago

If a base class has a public non-virtual destructor, then unique pointers to derived classes shouldn't be convertible (automatically or explicitly) to unique pointers of the base class.

That might be hard to enforce while using std::unique_ptr, so cpp2 should probably implements its own unique_ptr