mardi 22 septembre 2020

Variadic content leads to AddressSanitizer: heap-use-after-free on address

Object of type AAAA can hold any other object depending on its category. For example it holds object of type BBBB. An object of type BBBB also can hold any object content depending on its category.

Address sanitizer fails for the following code:

#include <assert.h>
#include <iostream>
#include <string>
#include <vector>

using namespace std;

class AAAA;
AAAA generate_a();

enum class A_CATEGORY
{
    Not_Seclected,
    B_DATA_CAT,
    // other categories
};

enum class B_CATEGORY
{
    Not_Seclected,
    C_DATA_CAT,
    // other categories
};

class CCCC
{
public:
    vector<int> needed_dummy;
};

class BBBB
{
public:
    B_CATEGORY category = B_CATEGORY::Not_Seclected;
    void * p_variadic_content = nullptr;

    BBBB()
    {
        cout<<"BBBB: default constructor"<<endl;
    }

    BBBB(const BBBB &obj)
    {
        *this = obj;
        if(obj.p_variadic_content != nullptr)
        {
            switch(this->category)
            {
                case B_CATEGORY::C_DATA_CAT:
                    this->p_variadic_content = (void *) new CCCC();
                    *((CCCC*) this->p_variadic_content) = *((CCCC*) obj.p_variadic_content);
                    cout<<"BBBB: copy constructor"<<endl;
                    break;
                default:
                    cout<<"Unknown category"<<endl;
            }
        }
    }

    ~BBBB()
    {
        switch(this->category)
        {
            case B_CATEGORY::C_DATA_CAT:
                assert(p_variadic_content != nullptr);
                delete (CCCC *) p_variadic_content;
                p_variadic_content = nullptr;
                cout<<"~BBBB"<<endl;
                break;
            default:
                cout<<"Unknown category"<<endl;
        }
    }
};

class AAAA
{
public:
    A_CATEGORY category = A_CATEGORY::Not_Seclected;
    void * p_variadic_content = nullptr;

    AAAA()
    {
       cout<<"AAAA: def constructor"<<endl;
    }

    AAAA(const AAAA &obj)
    {
        *this = obj;
        if(obj.p_variadic_content != nullptr)
        {
            switch(this->category)
            {
                case A_CATEGORY::B_DATA_CAT:
                    this->p_variadic_content = (void *) new BBBB();
                    *((BBBB*) this->p_variadic_content) = *((BBBB*) obj.p_variadic_content);
                    cout<<"AAAA: copy constructor"<<endl;
                    break;
                default:
                    cout<<"Unknown category"<<endl;
            }
        }
    }

    ~AAAA()
    {
        switch(this->category)
        {
            case A_CATEGORY::B_DATA_CAT:
                assert(p_variadic_content != nullptr);
                delete (BBBB *) p_variadic_content;
                p_variadic_content = nullptr;
                cout<<"~AAAA"<<endl;
                break;
            default:
                cout << "Unknown category" << endl;
        }
    }
};

AAAA generate_a()
{
    AAAA a_object;
    a_object.category = A_CATEGORY::B_DATA_CAT;
    BBBB * b_object = new BBBB();
    CCCC * c_object = new CCCC();
    b_object->category = B_CATEGORY::C_DATA_CAT;
    b_object->p_variadic_content = (void *) c_object;
    a_object.p_variadic_content = (void *) b_object;
    return a_object;
}

int main()
{
    vector<AAAA> aaa;
    AAAA a_object = generate_a();
    aaa.push_back(std::move(a_object));

    return 0;
}

compiled by

g++ -std=c++11 -fsanitize=address  aaaa.cpp  && ./a.out

The execution fails with the following trace

AAAA: def constructor
BBBB: default constructor
BBBB: default constructor
AAAA: copy constructor
~BBBB
~AAAA
=================================================================
==14684==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300000efe8 at pc 0x000000401ce6 bp 0x7ffe95d188c0 sp 0x7ffe95d188b0
READ of size 8 at 0x60300000efe8 thread T0
    #0 0x401ce5 in std::vector<int, std::allocator<int> >::~vector() (./a.out+0x401ce5)
    #1 0x401701 in CCCC::~CCCC() (./a.out+0x401701)
    #2 0x4017da in BBBB::~BBBB() (./a.out+0x4017da)
    #3 0x401c00 in AAAA::~AAAA() (./a.out+0x401c00)
    #4 0x402b0c in void std::_Destroy<AAAA>(AAAA*) (./a.out+0x402b0c)
    #5 0x4027fc in void std::_Destroy_aux<false>::__destroy<AAAA*>(AAAA*, AAAA*) (./a.out+0x4027fc)
    #6 0x4023b5 in void std::_Destroy<AAAA*>(AAAA*, AAAA*) (./a.out+0x4023b5)
    #7 0x40204e in void std::_Destroy<AAAA*, AAAA>(AAAA*, AAAA*, std::allocator<AAAA>&) (./a.out+0x40204e)
    #8 0x401dcd in std::vector<AAAA, std::allocator<AAAA> >::~vector() (./a.out+0x401dcd)
    #9 0x4014d4 in main (./a.out+0x4014d4)
    #10 0x7fa55d31083f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
    #11 0x401118 in _start (./a.out+0x401118)

0x60300000efe8 is located 8 bytes inside of 24-byte region [0x60300000efe0,0x60300000eff8)
freed by thread T0 here:
    #0 0x7fa55dd4eb8a in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99b8a)
    #1 0x4017e2 in BBBB::~BBBB() (./a.out+0x4017e2)
    #2 0x401c00 in AAAA::~AAAA() (./a.out+0x401c00)
    #3 0x4014c8 in main (./a.out+0x4014c8)
    #4 0x7fa55d31083f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)

previously allocated by thread T0 here:
    #0 0x7fa55dd4e592 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99592)
    #1 0x40126b in generate_a() (./a.out+0x40126b)
    #2 0x401498 in main (./a.out+0x401498)
    #3 0x7fa55d31083f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)

SUMMARY: AddressSanitizer: heap-use-after-free ??:0 std::vector<int, std::allocator<int> >::~vector()
Shadow bytes around the buggy address:
  0x0c067fff9da0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9db0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9dd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c067fff9df0: fa fa fa fa fa fa fa fa fa fa fa fa fd[fd]fd fa
  0x0c067fff9e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff9e40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==14684==ABORTING

So,

1- What is wrong?

2- How should I fix it without changing the structure? I am after a solution by correcting the memory management rather than getting around the problem. I am mainly after a fix in constructors/destructor. Also, I fill extend the categories. So, I am not after removing them. I will not accept removing void* or try replacing it by an explicit type.

Aucun commentaire:

Enregistrer un commentaire