Markakd / safe_tcmalloc

Apache License 2.0
0 stars 0 forks source link

STL vector 0-byte overlap #2

Open kitaharazy opened 2 years ago

kitaharazy commented 2 years ago

Basically, in cnedfunction.cc, void cNEDFunction::parseSignature(const char *signature)

static bool splitTypeAndName(const std::string& pair, char& type, std::string& name)
{
    std::vector<std::string> v = StringTokenizer(pair.c_str()).asVector();
    if (v.size()!=2)
        return false;
    type = parseType(v[0]);
    name = v[1];
    return type!=0;
}

void cNEDFunction::parseSignature(const char *signature)
{
    ......
    minargc = -1;
    std::vector<std::string> args = StringTokenizer(argList.c_str(), ",").asVector();  // A vector is created here
    for (int i=0; i < (int)args.size(); i++)
    {
        char argType;
        std::string argName;
        if (!splitTypeAndName(args[i], argType, argName))     // Another vector will be created inside the splitTypeAndName
            throw cRuntimeError(syntaxErrorMessage, signature);
        argtypes += argType;
        if (contains(argName,"?") && minargc==-1)
            minargc = i;
    }
    maxargc = argtypes.size();
    if (minargc==-1)
        minargc = maxargc;
}

In the end of splitTypeAndName, ~vector -> ~_Vector_base will finally be called to _M_deallocate,

At this time, the std::vector<std::string> v create in splitTypeAndName is going to be deallocated,

however, when the local std::vector<std::string> v being deallocated:

v._M_impl  # in splitTypeAndName
{
    _M_start = 0x147d7fa20400
    _M_finish = 0x147d7fa20440,
    _M_end_of_storage = 0x147d7fa20440
}

args._M_impl  # in parseSignature
{
    _M_start = 0x147d7fa203c0,
    _M_finish = 0x147d7fa20400,
    _M_end_of_storage = 0x147d7fa20400
}

Notice that v._M_start = 0x147d7fa20400 which equals to args._M_impl._M_finish = 0x147d7fa20400

Because of that when v._M_start = 0x147d7fa20400 is poisioned, it will affect args._M_impl like:

pwndbg> x /3gx 0x7fffffffdd88 # vector v
0x7fffffffdd88: 0xdeadbeefdeadbeef  0x0000147d7fa20440
0x7fffffffdd98: 0x0000147d7fa20440

pwndbg> x /3gx 0x7fffffffde38 # vector args
0x7fffffffde38: 0x0000147d7fa203c0  0xdeadbeefdeadbeef
0x7fffffffde48: 0xdeadbeefdeadbeef

which means that args._M_impl._M_finish = args._M_impl._M_end_of_storage = v._M_impl._M_start = 0cdeadbeefdeadbeef

After we reach the end of parseSignature, ~vector will also be called to deallocate std::vector<std::string> args But because the poison, the args is like:

args._M_impl 
{
    _M_start = 0x147d7fa203c0,
    _M_finish = 0xdeadbeefdeadbeef,
    _M_end_of_storage = 0xdeadbeefdeadbeef
}

Which is obviously invalid.

The escape bug in this case can be triggered and expounded through the gdb script:

# Add -g flag to v++, and run specmake clean && specmake all, in 520's build dir
# save this as a.gdb
# exec: gdb -q ./omnetpp_r_base.violet in ${YOUR_SPEC}/benchspec/CPU/520.omnetpp_r/run/run_base_*.0000
# and source a.gdb
b simulator/cnedfunction.cc:124

run "-c General -r 0 > omnetpp.General-0.out"

set $vec_args = args
set $args = (void *)((char *)&args+8)

b __gnu_cxx::__alloc_traits<std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::deallocate if (*(unsigned long *)$args) == __p

# /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/alloc_traits.h:141
cla7aye15I4nd commented 2 years ago

I am not sure if I understand the bug. In my option, the root cause is because one chunk's start address is another chunk's end address. It means the two chunk have a "0-byte overlap". The bug is crazy, it may means we can not use tcmalloc, because the memory allocated by tcmalloc is completely contiguous but ptmalloc not.

cla7aye15I4nd commented 2 years ago

I have another method, we can reserve 8-byte address at the end of chunk at any time. Then chunks in memory will not be contiguous.

cla7aye15I4nd commented 2 years ago

We can even use the reserved area as 'heap canary`.

kitaharazy commented 2 years ago

How this overlap happened: link

kitaharazy commented 2 years ago

reproduction case:

compile it by v++

#include<vector>
#include<iostream>
char data[0x9] = "aaaaaaaa";

void trigger(){

    std::vector<std::string> v;
    v.push_back(std::string(data));
    printf("\nv._M_impl._M_start: %#lx\nv._M_impl._M_end: %#lx\nv._M_impl._M_end_of_storage: %#lx\n\n",*(unsigned long *)((char *)&v+0),*(unsigned long *)((char *)&v+8),*(unsigned long *)((char *)&v+0x10));    

    std::vector<std::string> v2;
    v2.push_back(std::string(data));
    printf("\nv2._M_impl._M_start: %#lx\nv2._M_impl._M_end: %#lx\nv2._M_impl._M_end_of_storage: %#lx\n\n",*(unsigned long *)((char *)&v2+0),*(unsigned long *)((char *)&v2+8),*(unsigned long *)((char *)&v2+0x10));  

}
int main(){
    std::vector<std::string> args;

    args.push_back(std::string(data));
    printf("\nargs._M_impl._M_start: %#lx\nargs._M_impl._M_end: %#lx\nargs._M_impl._M_end_of_storage: %#lx\n\n",*(unsigned long *)((char *)&args+0),*(unsigned long *)((char *)&args+8),*(unsigned long *)((char *)&args+0x10));

    trigger();

}
kitaharazy commented 2 years ago

In ptmalloc, this kind of vector overlap won't happen, in violet it will happen. I think the reason why the bug won't happen in ptmalloc is that ptmalloc will add extra bytes for each chunk's control struct, so two chunk allocate by malloc() won't be 0-byte overlap. Therefore, maybe we can try reserving some bytes, also can be used as heap canary, at the end of each chunk as what Zheng said.

violet:

args._M_impl._M_start: 0x147d7fa183e0
args._M_impl._M_end: 0x147d7fa18400
args._M_impl._M_end_of_storage: 0x147d7fa18400

v._M_impl._M_start: 0x147d7fa18000
v._M_impl._M_end: 0x147d7fa18020    // *
v._M_impl._M_end_of_storage: 0x147d7fa18020    // *

v2._M_impl._M_start: 0x147d7fa18020    // *
v2._M_impl._M_end: 0x147d7fa18040
v2._M_impl._M_end_of_storage: 0x147d7fa18040

native clang++

args._M_impl._M_start: 0x417eb0
args._M_impl._M_end: 0x417ed0
args._M_impl._M_end_of_storage: 0x417ed0

v._M_impl._M_start: 0x4182f0
v._M_impl._M_end: 0x418310    // *
v._M_impl._M_end_of_storage: 0x418310    // *

v2._M_impl._M_start: 0x418320    // *
v2._M_impl._M_end: 0x418340
v2._M_impl._M_end_of_storage: 0x418340
Markakd commented 2 years ago

The escape is meant to be simple and light, we shouldn’t make it too complex.

On Sun, Oct 9, 2022 at 13:40 dataisland @.***> wrote:

We can even use the reserved area as 'heap canary`.

— Reply to this email directly, view it on GitHub https://github.com/Markakd/safe_tcmalloc/issues/2#issuecomment-1272603979, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEOMNZO2XIHBZWPNHNNSBRTWCMGSDANCNFSM6AAAAAARAYKGA4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>