ARPSpoofing: fix out of memory in one elements size vectors

[dsxack]

fix for #38 issue

[oliviertilmans]

Hello,

Thanks for the bug hunting!

This will however not completely fix the issue, as cases where multiple MAC addresses are involved will still crash with that patch.

The bug is triggered by this case (in any of the loops containing an erase):

• The MAC address we're looking for is the last one in the vector
• We erase it, and set the iterator to a valid next one as returned by the erase method, thus an iterator in [begin=element(0), element(1), .. element(size()-1), end] -- in this case vector.end()
• We then increment the value of it past end(), which is undefined behavior. In this case, we cause an infinite loop as it will never be == to end(), and even worse, we cast an undefined pointer (it) and perform a string equality comparison on an arbitrary memory zone (we implicitly try to construct a std::string from the memory area pointed to by *it when dereferencing it, hence the bad_alloc exception because the implicit new failed)

The check for size() that you added in solves this problem in the case where the vector is only one-element long.

If you want to patch this using the size() way, I'd compare it the already tracked count variables around, but that's an opaque way to check for this edge case (which would need to be commented/documented etc).

I'd instead simply check after the erase() calls (still inside the if's) that it is not the end() of the vector and break out if that's the case, à-là

for(it = TargetMACs->begin() ; it != TargetMACs->end() ; it++) {
        if( (*it) == AttackerMAC ) {
            it = TargetMACs->erase(it);
            /* And erase it from IP list */
            TargetIPs->erase(TargetIPs->begin() + count);
            if (it == TargetMACs->end())
                /* We deleted the last element, stop here. */
                break;
        }
        count++;
    }

Your 3rd change is in the wrong loop btw, as it should be for the innerloop (where the erase happens), and not the outer one (where when size() == 0 you have begin() == end() anyway)

[dsxack]

Thank you, I commit updated according to your recommendations

[oliviertilmans]

Thanks!