7
\$\begingroup\$

Intro

This time I was in the mood for the low level stuff. The entire repository is in GitHub. It includes the installer, the actual malware program, and the uninstaller that removes it completely from your system. The malware program does nothing more severe than simply showing 3 message boxes - one by one - and exits all 3 were presented. The malware installer copies the actual malware program to C:\Windows\system32 and adds a registry key SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Run\\SampleMalware with the value pointing to the system32. Also, the package contains the uninistaller that removes the file from the system folder and deletes the aforementioned registry key.

Code

SampleMalware.cpp:

#include <Windows.h>
int WINAPI WinMain(HINSTANCE hInstance, 
 HINSTANCE hPrevInstance,
 PSTR lpCmdLine, 
 int nCmdShow) {
 for (int i = 0; i < 3; i++) {
 MessageBoxW(NULL, L"This is SampleMalware.exe!", L"", MB_OK);
 }
 
 return 0;
}

SampleMalwareInstaller.cpp:

#include <Windows.h>
#include <fstream>
#include <iostream>
using std::ofstream;
void CopyFile() {
 std::ifstream ifs;
 std::ofstream ofs;
 ifs.open("SampleMalware.exe", std::ios::binary);
 ofs.open("C:\\Windows\\system32\\SampleMalware.exe", std::ios::binary);
 char c;
 while (true) {
 c = ifs.get();
 
 if (ifs.eof()) {
 break;
 }
 ofs.put(c);
 }
 ifs.close();
 ofs.close();
}
int WINAPI WinMain(HINSTANCE hInstance,
 HINSTANCE hPrevInstance,
 LPSTR lpCmdLine,
 int nCmdShow)
{
 // Install to registry:
 HKEY hKey;
 LSTATUS lStatus1 = 
 RegOpenKeyExW(HKEY_LOCAL_MACHINE, 
 L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Run", 
 0, 
 KEY_ALL_ACCESS,
 &hKey);
 if (lStatus1 != ERROR_SUCCESS) {
 return 1;
 }
 WCHAR path[] = L"%windir%\\system32\\SampleMalware.exe";
 LSTATUS lStatus2 =
 RegSetValueExW(hKey,
 L"SampleMalware",
 0, 
 REG_EXPAND_SZ,
 (LPBYTE) path,
 sizeof(path)); 
 if (lStatus2 != ERROR_SUCCESS) {
 RegCloseKey(hKey);
 return 2;
 }
 RegCloseKey(hKey);
 // Copy SampleMalware.exe to %windir%\system32\SampleMalware.exe:
 CopyFile();
 return 0;
}

SampleMalwareUninstaller.cpp:

#include <Windows.h>
#include <iostream>
int WINAPI WinMain(HINSTANCE hInstance,
 HINSTANCE hPrevInstance,
 PSTR lpCmdLine,
 int nCmdShow) {
 remove("C:\\Windows\\system32\\SoftwareMalware.exe");
 RegDeleteKeyW(
 HKEY_LOCAL_MACHINE,
 L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Run\\SampleMalware");
 return 0;
}

Critique request

As always, I would like to hear any comment regarding my attempt.

asked Jun 7 at 7:53
\$\endgroup\$

4 Answers 4

16
\$\begingroup\$
  • Don't write malware.
  • Consider using an installer creator like NSIS instead of writing your own.
  • Don't hardcode paths. Windows might not even be installed in C:\Windows. Use a function like GetSystemDirectoryA() to get the actual path of the system directory.
  • Inconsistent use of paths: apart from having both SampleMalware.exe and SoftwareMalware.exe, %windir% is not necessarily the same as C:\Windows.
  • Missing error checking: you only check for errors from the registry functions in the installer, but you don't check anything else.
  • Potential infinite loop in CopyFile(): ifs.eof() will only ever be true if the end of the file has been reached succesfully. If an error occurs before that, it might never become true. Use ifs.good() to check if you can still read from the file. Or even better:
  • Use std::filesystem::copy() to copy files and std::filesystem::remove() to delete them.
  • The registry key could be used by the uninstaller as well to find the path used at installation time.
  • Missing #includes: remove() is part of <cstdio>, not of <iostream>.
  • Return EXIT_SUCCESS and EXIT_FAILURE from main(); other return values are rarely useful.
answered Jun 7 at 9:51
\$\endgroup\$
0
7
\$\begingroup\$

Additionally to the points listed by G. Sliepen:

  1. As you don't check the result when calling close on std::ifstream and std::ofstream, close is unnecessary, as RAII.

  2. Your indentation in SampleMalware.cpp at the following block is wrong:

    int WINAPI WinMain(HINSTANCE hInstance, 
     HINSTANCE hPrevInstance,
     PSTR lpCmdLine, 
     int nCmdShow) 
    

    and inconsistent with the (correct) indentation you use in later in SampleMalwareUninstaller.cpp.

  3. lStatus1 and lStatus2 are rather poor names for variables.

  4. lStatus1 and lStatus2 are set once and never changed. Declare them as const to make this explicit.

  5. WinMain in SampleMalwareInstaller.cpp is doing too much—the code that deals with Registry should be moved to a separate function (and maybe even multiple functions).

  6. The types of several variables can be deduced by the compiler. Make them auto.

  7. As a matter of habit, use ++i, not i++. Unless you specifically want a different behavior, you should increment something first, and only then return it. Not the other way around.

answered Jun 7 at 19:33
\$\endgroup\$
7
\$\begingroup\$

The code has already been commented by other answers. My review focuses on your development process around this attempt.

To anyone attempting to run the executables: do it in a VM only. Disconnect the VM from the network and reset the VM to a snapshot afterwards. You don't know what those executables are doing.

  1. Don't upload executables into a Github project as part of the source code. Instead, create a release with the utilities given by Github and attach the executables there.

  2. It does not work. Have you tried running it? On my system, SampleMalware.exe is not copied to the system32 directory and there's no Registry key being written. Probably the executables are missing administrator privileges. I am using the executables you provided on Github. Their manifest file says, it should be run "asInvoker".

  3. It still does not work when I run the executables as Administrator. That's because Windows Defender detects the executables as "Trojan:Script/Conteban.A!ml". If you write malware, check whether it's detected or not, using several virus scanners, Windows Defender being the first one, since everybody has it.

  4. The malware is a separate executable. The installer is quite useless if the malware is not present. If people pass on the installer (which you want), they might not have enough technical knowledge to pass the malware with it. Therefore, you want to embed the malware into the installer, so everything is a self-contained package.

  5. There's another reason to do the same thing: As of now, the malware is an executable which is located right beside the installer. That way, it can be easily detected by virus scanners. You don't want the malware be detected before it is installed.

answered Jun 8 at 11:19
\$\endgroup\$
0
3
\$\begingroup\$

Unless you're trying to support Windows XP, you can look into RegSetKeyValueW and decide whether you want to use that.

answered Jun 8 at 4:50
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.