Stack around variable 'temp' is corrupted

Question

I'm trying to convert an std::string containing multiple values separated by spaces into a vector object. I have it working (meaning the debugger shows the correct values in the vector at the end of the algorithm), however I'm getting an exception that I've never seen before. The exception is below:

I've never seen this before, and I've been researching for an hour as to what is causing this and I still don't understand it.

My algorithm that's causing the exception is as follows:

vector<BYTE> ConvertStringOfValuesToVectorOfBytes(std::string valStr)
{
    vector<string> vectorOfValues = split(valStr, ' '); //split string into vector of strings (works)
    vector<BYTE> Hex;
    vector<const char*> vectOfCStrings;

    for(int i = 0; i < vectorOfValues.size(); i++)
    {
        const char* temp = vectorOfValues[i].c_str();
        vectOfCStrings.push_back(temp);
    }

    //now we have a vector of c strings...

    for(int i = 0; i < vectOfCStrings.size(); i++)
    {       
        char temp = 0;
        sscanf(vectOfCStrings[i], "%x", &temp);
        Hex.push_back(temp);
    }

    return Hex;

} //<-- debugger gets to here and on the next step causes the exception

I have no clue as to how I can fix this problem. As I said, the algorithm "works" in the sense that I'm getting the right result, I just need to clear up the exception. How do I fix this?

Answer 1

What exactly is your algorithm trying to accomplish?

From the looks of it, you are breaking a std::string into a std::vector of 'words', and then converting those words into c-style strings (why?), then assembling a std:vector containing the first letter of each word interpreted as an int. Use a std:stringstream instead:

for ( auto& word : vectorOfValues )
{
    for ( auto& letter : word ) 
    {
        unsigned int val;
        std::stringstream ss;
        ss << std::hex << word[0];
        ss >> val;
        Hex.push_back( static_cast<BYTE>( val ) );
    }
}

Generally, if your not directly interfacing with a C API, you shouldn't be using c-style strings in c++.

Answer 2

std::vector<unsigned char> func(std::string valStr) {
    vector<string> vectorOfValues = split(valStr); //split string into vector of strings (works)
    vector<BYTE> Hex;
    vector<const char*> vectOfCStrings;

    for(int i = 0; i < vectorOfValues.size(); i++) {
        const char* temp = vectorOfValues[i].c_str();
        vectOfCStrings.push_back(temp);
    }

    //now we have a vector of c strings...

    for(int i = 0; i < vectOfCStrings.size(); i++) {
        int temp = 0;
        sscanf(vectOfCStrings[i], "%x", &temp);
        Hex.push_back(char(temp));
    }

    return Hex;
}

So yeah, I just made the second tmp an int, because sscanf and stuff like that use int as types, and since a byte is smaller than an int, that cause your overflow.

Not sure if it works, but it doesn't give me the error any more.

Answer 3

This line is not right:

sscanf(vectOfCStrings[i], "%x", &temp);

The format %x is to meant to be used for an int, not a char.

You are getting stack corruption because sscanf expects an address of an int. It tries to use the address you pass as being capable of holding an int. It obviously uses more memory than is valid.

What you need is:

for(int i = 0; i < vectOfCStrings.size(); i++)
{       
    int temp = 0;
    sscanf(vectOfCStrings[i], "%x", &temp);
    Hex.push_back(static_cast<char>(temp));
}

Answer 4

Culprit seems to be in these 2 lines:

    char temp = 0;
    sscanf(vectOfCStrings[i], "%x", &temp);

This exactly why printf/scanf family of functions are not used in C++ and were replaced by std::iostream mechanism. Function sscanf expects to get pointer to unsigned int when you specify %x but you pass pointer to char, and there is no way sscanf itself can validate that. Though some modern compilers can check if values passed to printf or scanf matches format specifiers and generate warnings otherwise, that does not work in general (for example format passed to a function, instead of using string literal). So here sscanf dereference pointer &temp as pointer to unsigned int and overwrites memory around temp variable as sizeof of char == 1.