0

I am having a hard time figuring out why the payload isn't working after decryption in example2.cpp, when executing the compiled exe with the command 'example2.exe > out.txt' I get the shellcode which is valid and does not cause any problem in example.cpp because I can see the output which is hello world (at least in my case)

example.cpp

unsigned char out[] = "\x00\x00...";
int main()
{
    void *exec = VirtualAlloc(0, sizeof(out), MEM_COMMIT, PAGE_EXECUTE_READWRITE);
    memcpy(exec, out, sizeof(out));
    ((void(*)())exec)();
}

example2.cpp

void decrypt_run(){
    std::vector<unsigned char> decrypted(encrypted.size());
    // the encrypted cipher get decrypted and the vector decrypted is filled with unsigned chars
    unsigned char buf[decrypted.size()];
    // converting the vector to an unsigned char buffer to be passed to memcopy 
    std::copy(decrypted.begin(), decrypted.end(), buf);
    size_t shellcodesize = sizeof(buf);
    cout << buf << endl;  // prints the shellcode to the screen 
    //void *exec = VirtualAlloc(0, shellcodesize, MEM_COMMIT, PAGE_EXECUTE_READWRITE);
    //memcpy(exec, buf, shellcodesize);
    //((void(*)())exec)();
}
int main()
{
    decrypt_run();
    return 0;
}

when uncommenting the last three lines in decrypt_run() the program finishes without any output other than the shellcode it self

Again the same shellcode in the out.txt is used with example.cpp and it runs flawlessly but not with example2.cpp

loaded_dypper
  • 262
  • 3
  • 12
  • 2
    `unsigned char buf[decrypted.size()];` -- This is not valid C++. Array sizes must be denoted by a compile-time expression, not a runtime value. The strange thing is that you actually did use what would be valid, `std::vector` at the line right before that one. – PaulMcKenzie Jan 21 '22 at 20:59
  • `sizeof(out)` -- Also, what exactly is `out`? Using `sizeof` looks suspicious to me. Is `out` a pointer? If so, then using `sizeof` is incorrect, as you will get the `sizeof` a pointer, which will be 4 or 8 depending on whether you're running a 32-bit or 64-bit app. – PaulMcKenzie Jan 21 '22 at 21:00
  • @PaulMcKenzie can you elaborate please , `out` is a buffer of unsigned chars in the first file i declared it as out in the second one as buf – loaded_dypper Jan 21 '22 at 21:07
  • Elaborate on what exactly? I basically explained what the issues are. If you mean the usage of the fake array syntax, then that should be: `std::vector buf(decrypted.size());` – PaulMcKenzie Jan 21 '22 at 21:07
  • you state that i used what would be right `std::vector` what do you mean by that – loaded_dypper Jan 21 '22 at 21:09
  • A `std::vector` is the dynamic array class in C++. Then on the very next line, you are attempting to declare -- a dynamic array, but it is a fake one. Just use `std::vector` again. Please see [why VLA's are not part of C++](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard). And in all of that, who the heck knows what this: `size_t shellcodesize = sizeof(buf);` will do, given that `buf` is not really C++. For vector, you would simply use `size()`. – PaulMcKenzie Jan 21 '22 at 21:11
  • @PaulMcKenzie i don't understand how and why would use a vector instead of a buffer to copy to the space allocated in virtualalloc – loaded_dypper Jan 21 '22 at 21:20
  • You are missing the point -- the code you posted is not valid C++. Take that code to Visual C++, and you will be greeted with a compiler error, and for good reason. Since it isn't valid C++, no one can make conclusions from it. Make it valid C++, and then things can go forward from there. Also, a `vector` is nothing more than a dynamic array, exactly like what you are trying to do with that code. – PaulMcKenzie Jan 21 '22 at 21:23
  • [See this](https://godbolt.org/z/665jbcW8v). – PaulMcKenzie Jan 21 '22 at 21:29
  • @PaulMcKenzie but compiling with g++ does not throw any errors – loaded_dypper Jan 21 '22 at 21:32
  • Did you read the link I provided that variable length arrays are not part of C++? And the reason why it "compiled" is because that code is an extension that is provided by that compiler. But it still is not valid C++. Look at the warning at the last link I posted. Why are you hesitant in simply changing the code to make it valid C++? This is one reason why I wished g++ would turn *off* the extensions, instead of leaving them on and fooling programmers into believing they're creating valid C++ code. – PaulMcKenzie Jan 21 '22 at 21:35
  • Another thing -- if `decrypted.size()` is big enough, you will blow out the stack memory if you used that invalid syntax. – PaulMcKenzie Jan 21 '22 at 21:39
  • @PaulMcKenzie you are right i was compiling with `g++ .cpp` and that didn't show any warnings – loaded_dypper Jan 21 '22 at 21:48
  • @loaded_dypper There is no need for the dynamic `buf` array at all. You can copy directly from the `decrypted` vector into the allocated `exec`, eg: `void *exec = VirtualAlloc(0, decrypted.size(), MEM_COMMIT, PAGE_EXECUTE_READWRITE); memcpy(exec, decrypted.data(), decrypted.size()); ((void(*)())exec)();` – Remy Lebeau Jan 22 '22 at 00:15
  • @loaded_dypper Shellcode is binary data, printing it as-is to the console makes no sense to do. What exactly does `out.txt` look like? How exactly is the shellcode being written to `out.txt`, and how exactly is it being loaded into `encrypted`? Without this info, we can't rule out the possibility that `encrypted` has faulty data to begin with, thus causing `decrypted` to have faulty data. – Remy Lebeau Jan 22 '22 at 00:25

1 Answers1

1

In the first case, you are hard-coding the shellcode in a fixed memory buffer, and then copying those bytes as-is into your allocated executable memory. Which is fine.

But in your second case, you have a dynamic vector filled with the (decrypted) shellcode bytes, and then you are allocating a dynamic array using non-standard VLA syntax, copying the shellcode into that array, and then copying the shellcode from that array into your executable memory. That intermediate array is completely unnecessary and should be removed, you can copy the shellcode bytes directly from the vector into the executable memory, eg:

void decrypt_run(){
    std::vector<unsigned char> decrypted(encrypted.size());
    // the encrypted cipher get decrypted and the vector decrypted is filled with unsigned chars
    // passing the vector to memcopy 
    unsigned char *buf = decrypted.data();
    size_t shellcodesize = decrypted.size();
    cout << buf << endl;  // prints the shellcode to the screen 
    void *exec = VirtualAlloc(0, shellcodesize, MEM_COMMIT, PAGE_EXECUTE_READWRITE);
    memcpy(exec, buf, shellcodesize);
    ((void(*)())exec)();
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • This is helpful but when compiling and viewing it in gdb I get SIGILL after the shellcode is printed to the console and still no output – loaded_dypper Jan 22 '22 at 11:18
  • @loaded_dypper first off, you can't visually print shellcode *as-is*, it is *binary* data. You would have to print an *encoded* form of it, such as hexadecimal. Second, if the decrypted shellcode is not running correctly, then there is likely a problem with the encryption/decryption. – Remy Lebeau Jan 22 '22 at 18:42
  • I am pretty sure the problem is not in the encryption/decryption because i use the same library in another project with works fine – loaded_dypper Jan 22 '22 at 20:44
  • 1
    @loaded_dypper given the code shown, either the shellcode is wrong to begin with, or the encryption/decryption is being used wrong and corrupting it. Since you did not provide a [mcve], there is no way for anyone here to know one way or the other. I would suggest removing the encryption and see if you can successfully load the shellcode from an unencrypted file first, then re-add the encryption. – Remy Lebeau Jan 22 '22 at 21:45
  • I did use the shellcode without encryption/decryption and it works fine – loaded_dypper Jan 22 '22 at 21:53
  • @loaded_dypper but did you load the unencrypted shellcode from a file, or did you just hard-code it? Two completely different things. IF you were able to load the shellcode from a file, and IF it could run correctly, then the problem would have to be with the encryption. – Remy Lebeau Jan 22 '22 at 22:03