0

I'm having trouble writing a WString to the STDIN of a child process. If I have only acii character string (eg: @WSX3edc), the code works fine, but if it contains a non ascii character (eg: @WSX3edcß) it fails.

The child process is 7zr.exe (7Zip cmd line version). The input I'm writing to the STDIN is the password to extract the file.

// inject password
wPassword.append(password);
wPassword.append(L"\n"); \\For carriage return
...
DWORD dwBytesToWrite = wPassword.length()*sizeof(wchar_t);
DWORD dwBytesWritten = 0;
char szBuffer[1024] = "\0";
wcstombs(szBuffer, wPassword.c_str(),wcslen(wPassword.c_str())+1);
dwBytesToWrite = strlen(szBuffer);
if (!WriteFile(hInput, szBuffer, dwBytesToWrite, &dwBytesWritten, NULL)) {
    std::cout<<"write file failed"<<GetLastError()<<std::endl;
    goto Cleanup;
}

The writefile always succeed but some how the file extraction is not successful due to faulty password injecting mechanism.

Createprocess for this looks like : (si object has the STDIN and STDOUT streams set using a CreatePipe earlier)

if(!CreateProcess((LPWSTR)cmd, (LPWSTR)cmdArgs, NULL, NULL, TRUE, NORMAL_PRIORITY_CLASS,
        NULL, NULL, &si, &pi)) {
         std::cout<<"7zr.exe process creation failed "<<GetLastError()<<std::endl;
         goto Cleanup;
}

Note : 7zr.exe works just fine with this particular password, if we run it on command-line and paste this password. The extraction works fine.

gkns
  • 697
  • 2
  • 12
  • 32

2 Answers2

2

If the narrow character set doesn't have the relevant password character, you can't use this approach. Instead find what option 7zr has for specifying the password. I don't have an executable called 7zr but I do have 7z, and the command 7z | find /i "pass" worked nicely.

In other news:

  • The variable dwBytesToWrite is initialized with one value, only to be reassigned a few lines later, without having been used.

  • goto Cleanup is generally ungood in C++. If you want guaranteed cleanup use a destructor (the technique called RAII, read up on it).

  • Microsoft's Hungarian notation, with prefixes such as sz and dw, is generally an abomination. It once, in the 1980's, supported the help system in Microsoft's Programmer's Workbench. AFAIK that product hasn't existed the last 30 years or so.

  • The C cast in (LPWSTR)cmd can easily introduce a bug. Use const_cast where you want to cast constness. Then it would be more clear that this cast is incorrect: you need a mutable buffer.

  • Instead of reporting a failure to the standard output stream, via std::cout, consider using the standard error stream, via either std::cerr or std::clog. Better, don't do i/o at the place where a failure is detected, but throw an exception to let the calling code deal with it. The calling code can't remove output that's already, uh, output.

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • If we add the password to the command by itself using -p, This password can be seen on process explorer tools. This is a security concern. hence directly echoing to the stdin. 7zr.exe works just fine with this particular password, if we run it on command-line and paste this password. The extraction works fine. – gkns Jan 12 '17 at 07:00
  • The security concern should be a separate question. Please don't invalidate current answers by adding that requirement to the question. But I'll answer it here, since it's so simple: if someone has controlling access to a computer, all bets are off wrt. a malicious attacker. All that remains is to guard against ordinary users stumbling over the password and using it inadvertently, breaking things. You can guard against that with a stern warning. Yep, that's one good answer. Because it's not really a security concern. Just like Microsoft UAC is an annoyance, not security but marketing. – Cheers and hth. - Alf Jan 12 '17 at 08:49
  • 1
    Oh. Note that the command line is UTF-16 encoded. The standard input stream is usually assumed, by the application, to be Windows ANSI encoded. Where Windows ANSI is the codepage specified by the `GetACP` API funciton. – Cheers and hth. - Alf Jan 12 '17 at 09:24
  • Thanks for that input! It worked after converting to OEM CP: WideCharToMultiByte(CP_OEMCP, WC_COMPOSITECHECK|WC_NO_BEST_FIT_CHARS, wPassword.c_str(), -1, mbBuffer, sizeof(mbBuffer), NULL, NULL); – gkns Jan 12 '17 at 11:58
1

wcslen(wide) returns the number of wide characters in its argument wide (see).

wcstombs(narrow,wide,len) writes no more than len bytes to narrow (see).

Now if we always had one wide character = one narrow character = one byte, it wouldn't have much sense to have two varieties of characters, would it?

As soon as you have a wide character that translates to more than one narrow character, there is undefined behaviour.

Since your szBuffer is of fixed size, you could just as well write

wcstombs(szBuffer, wPassword.c_str(), sizeof(szBuffer));
n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
  • no luck. with the sizeof(szBuffer) modification. Also I didn't understand the undefined behavior, you mean we aren't supposed to attempt anything like this? – gkns Jan 12 '17 at 09:07
  • There is more than one problem in your code, I only pointed out one that the other answer doesn't mention. – n. m. could be an AI Jan 12 '17 at 14:10