-1

I have a program that calls SHGetKnownFolderPath with FOLDERID_RoamingAppData.

If I start the program by double clicking it, it works ok.

If the program is started by a windows service (in the current user context), the function fails with error E_ACCESSDENIED (-2147024891).

This is what my code looks like:

Tstring EasyGetFolderPath(REFKNOWNFOLDERID folderid)
{
    Tstring sPath = _T("");
    PWSTR pszPath = NULL;

    HRESULT hr = SHGetKnownFolderPath(folderid, 0, NULL, &pszPath);

    if (hr == S_OK && pszPath)
    {
        sPath = WStringToTCHAR(pszPath);
        CoTaskMemFree(pszPath);
        return sPath;
    }
    else
    {
        throw HResultException(hr, _T("SHGetKnownFolderPath failed"));
    }
}

Tstring EasyGetUsrAppDataPath()
{
    return EasyGetFolderPath(FOLDERID_RoamingAppData);
}

static TCHAR* WStringToTCHAR(const std::wstring &s)
{
#ifdef UNICODE
    TCHAR *sT = new TCHAR[s.length() + 1];
    _tcscpy_s(sT, s.length() + 1, s.c_str());
    return sT;
#else
    std::string str = WStringToString(s);
    TCHAR *sT = new TCHAR[str.length()+1];
    _tcscpy_s(sT, str.length() + 1, str.c_str());
    return sT;
#endif // UNICODE
}

static std::string WStringToString(const std::wstring& s, bool method = true)
{
    std::string temp;
    temp.assign(s.begin(), s.end());
    return temp;
}

This is the code that starts the process in the current user context: (I've removed the error handling in order to reduce verbosity)

void StartProcessInCurrentUserContext(const Tstring &sExeName, const Tstringarr &lstParams, const Tstring &sWorkingDir)
{
    ...

    EnableDebugPrivilege();

    errCode = GetProcessByName(_T("explorer.exe"), hProcess);

    if (!OpenProcessToken(hProcess, TOKEN_ALL_ACCESS, &hToken))
    {
        ...
    }

    if (hProcess)
        CloseHandle(hProcess);

    Tstring sCmdLine = ...;

    ...

    // Create the child process. 
    bSuccess = CreateProcessAsUser(hToken, NULL,
        (LPTSTR)sCmdLine.c_str(),            // command line 
        NULL,          // process security attributes 
        NULL,          // primary thread security attributes 
        TRUE,          // handles are inherited 
        0,             // creation flags 
        NULL,          // use parent's environment 
        sWorkingDir.length() > 0 ? (LPCTSTR)sWorkingDir.c_str() : NULL,
        &siStartInfo,  // STARTUPINFO pointer 
        &piProcInfo);  // receives PROCESS_INFORMATION 

    CloseHandle(hToken);

    ...
}

Does anyone know what the problem might be?

conectionist
  • 2,694
  • 6
  • 28
  • 50
  • Maybe User Access Control, a.k.a. UAC? Anyway why are you using the silly `_T` stuff? That's 16 years obsolete. – Cheers and hth. - Alf May 30 '16 at 15:08
  • Well, when I start it by double clicking, the UAC doesn't show up. So, I don't think it has anything to do with that. – conectionist May 30 '16 at 15:12
  • Possibly the problem lies in `WStringToTCHAR`. Show its definition. – Cheers and hth. - Alf May 30 '16 at 15:12
  • I edited the post. – conectionist May 30 '16 at 15:20
  • 1
    @conectionist You could avoid the memory leak if you used `std::vector` instead of `new TCHAR[]`. – PaulMcKenzie May 30 '16 at 15:25
  • You're right. Thanks for that. – conectionist May 30 '16 at 15:30
  • @PaulMcKenzie: What memory leak? The `new`-ed buffer is returned all the way up, out of the presented code. I see a possible memory leak if the dynamic allocation fails, but that doesn't matter. – Cheers and hth. - Alf May 30 '16 at 15:32
  • @Cheersandhth.-Alf -- You're right. Should say "potential memory leak". – PaulMcKenzie May 30 '16 at 15:36
  • I appreciate this, but can we please focus on the question at hand? – conectionist May 30 '16 at 15:41
  • Well, could you focus your minimal but complete example on the question at hand? For example, should we guess what `TCHAR` resolves to during compilation? – Ulrich Eckhardt May 30 '16 at 15:43
  • TCHAR is from winapi. Yes, it resolves at compile time. Like I said, the code works perfectly when I run the program manually. The problem is when the Windows service starts it. – conectionist May 30 '16 at 15:46
  • 1
    You say you launch your executable from the service "as the current user". How exactly do you do this (best to show the relevant `CreateProcess(WithLogon)` code). IOW maybe the started process does not run like you think it should (for example without `LOGON_WITH_PROFILE` - I could image that being a potential problem). – Christian.K May 30 '16 at 15:47
  • I've added the code (the relevant parts, at least). – conectionist May 30 '16 at 16:09
  • The meaning of TCHAR is defined via preprocessor switches at compile time. Since the switches are not given, you leave us to guess which code actually "works perfectly". Seriously, why don't you just go and read the posting guidelines, they don't just explain what a minimal example is but you could also learn why it's important! – Ulrich Eckhardt May 30 '16 at 16:19
  • There are lots of gotchas in the [CreateProcessAsUser](https://msdn.microsoft.com/en-us/library/windows/desktop/ms682429(v=vs.85).aspx) documentation - incorrect environment variables when you inherit them which could easily trigger this issue; lpDesktop needs to be set to ensure the process runs in the correct interactive window station. – Anya Shenanigans May 30 '16 at 16:26
  • 5
    "CreateProcessAsUser does not load the specified user's profile into the HKEY_USERS registry key." You need load the profile in order to access user profile state. – Raymond Chen May 30 '16 at 16:37
  • 1
    @conectionist -- This one line is **highly** suspicious: `(LPTSTR)sCmdLine.c_str(),`. If you took away the `(LPTSTR`) cast, do you get a compiler error? If so, then your code is not correct. You don't turn narrow strings into wide strings and vice-versa by casting. This goes back to what Ulrich was mentioning and the `TCHAR` issue. – PaulMcKenzie May 30 '16 at 20:11
  • 2
    You should use `WTSQueryUserToken()` instead of `GetProcessByName()`/`OpenProcessToken()`. And also use `CreateEnvironmentBlock()` for the `lpEnvironment` parameter, don't inherit the service's environment. – Remy Lebeau May 30 '16 at 20:54
  • @PaulMcKenzie: That cast does not try to convert character encodings. It is required, because `CreateProcessAsUser` requires, that the command line be a **writable** character array (for the Unicode version anyway). While not correct by the letter of the law, it is usually safe to do so. – IInspectable May 30 '16 at 20:57
  • @IInspectable - Let's see the compiler error when that cast is removed and exactly what `TString` and `c_str()` are. Until then, the cast is suspicious. – PaulMcKenzie May 30 '16 at 21:40
  • @PaulMcKenzie: The error is, that the compiler cannot convert from *pointer to const * to *pointer to *. If you want to point out complete bogus conversions, you should name `WStringToString` and `WStringToTCHAR`. Those are beyond hope of regeneration. Casting away constness - in contrast - is hardly a minor offence. – IInspectable May 30 '16 at 21:46
  • There's no obvious cause, though I concur with Petesh's comments. A [mcve] might help. – Harry Johnston May 31 '16 at 04:31
  • 1
    The question is tagged C++. If you're going to cast away constness, at least use a `const_cast`. – Cody Gray - on strike May 31 '16 at 09:59
  • @RaymondChen This seems to be cause. Please create an answer, so I can mark it as accepted. – conectionist May 31 '16 at 12:28
  • @RemyLebeau Thanks for the suggestion. – conectionist May 31 '16 at 12:29

1 Answers1

1

The documentation for SHGetKnownFolderPath says in the discussion of the hToken parameter:

In addition to passing the user's hToken, the registry hive of that specific user must be mounted.

The documentation for CreateProcessAsUser says

CreateProcessAsUser does not load the specified user's profile into the HKEY_USERS registry key.

These two paragraphs together explain why your code is not working. Fortunately, the next sentence in the documentation for CreateProcessAsUser explains what you need to do:

Therefore, to access the information in the HKEY_CURRENT_USER registry key, you must load the user's profile information into HKEY_USERS with the LoadUserProfile function before calling CreateProcessAsUser. Be sure to call UnloadUserProfile after the new process exits.

Raymond Chen
  • 44,448
  • 11
  • 96
  • 135
  • I don't think this can be the problem with the posted code - the token is taken from a copy of `explorer.exe` so the user's profile should already be loaded. – Harry Johnston May 31 '16 at 22:37
  • @HarryJohnston There are two problems. One is not passing the wrong token as the hToken parameter to SHGetKnownFolderPath. The other (apparently not relevant here, but relevant in general) is loading the profile. – Raymond Chen May 31 '16 at 23:56
  • The default is to use the current user's token, so that part should be OK too. But perhaps the security permissions on the process aren't right, since it was created from a different context, so when SHGetKnownFolderPath goes to fetch the current token it fails? A bit like https://blogs.msdn.microsoft.com/oldnewthing/20160512-00/?p=93447 ? – Harry Johnston Jun 01 '16 at 00:02
  • @HarryJohnston Oops, I misread. I thought the SHGetKnownFolderPath call was in the service. – Raymond Chen Jun 01 '16 at 00:04