New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 1644
Owner:
Closed: Nov 17
Cc:

Blocking:
issue 1644



Sign in to add a comment
link

Issue 1648: Windows: DfMarshal CFileStream::InitWorker Handle Duplication TOCTOU Elevation of Privilege

Reported by forshaw@google.com, Aug 24 Project Member

Issue description

Windows: DfMarshal CFileStream::InitWorker Handle Duplication TOCTOU Elevation of Privilege
Platform: Windows 10 1803 (not tested earlier, although code looks similar on Win8+)
Class: Elevation of Privilege

Note, this is a report on a single issue class in the DfMarshal unmarshaler. For background see the master issue.

Summary: During unmarshaling of a storage object it’s possible to win a race condition to change control flow to get an arbitrary handle duplicated to an untrusted process leading to EoP.

Description:

The storage implementation uses some complicated code to ensure the shared state is kept up to date. One of these is when a file stream is created the handle to that file will be pre-duplicated to all other sharers of the document so that they can automatically use it without having to go to the hassle of opening a process. This is only supposed to be used when creating a new file stream, however the code double fetches values from shared memory to determine control flow which can be raced. 

The code in question is in CFileStream::InitWorker which is called during unmarshaling:

HRESULT CFileStream::InitWorker(const wchar_t *pwcsPath, 
                                unsigned int dwFSInit, 
                                void *pSecurityDescriptor)
{
  WCHAR pwszTempFileName[MAX_PATH];
  WCHAR pwszFullPath[MAX_PATH];
  HRESULT hr;
  CGlobalFileStream* pgfst = this->_pgfst;

  if (pgfst->_awcPath[0]) <-- First check of path.
  {
    // Duplicate case.
    Init_DupFileHandle();
  }
  else
  {
    // Create file case.
    StgpGetTempFileName(pwszTempFileName);
    Init_OpenOrCreate(pwszTempFileName);
  }

  if (pgfst->_awcPath[0]) <-- Second check.
  {
    // We're done.
    return S_OK;
  }
  
  if (GetFullPathName(pwszTempFileName, MAX_PATH, pwszFullPath))
  {
    StringCbCopyW(pgfst->_awcPath, MAX_PATH, pwszFullPath);
    if (pgfst->_dwStartFlags & 0x40 )
      DupFileHandleToOthers();
    }
  }
  // ...
}

We can see that there’s first a check whether the global file stream path has a length > 0 (by just checking for the first character being 0). As long as the path has been specified then the handle is first duplicated from the original process into the process doing the unmarshal. If that succeeds then we get a second check of the path string. However, as the path string is in shared memory then an attacker can race the value of the path and change it from a non-zero value to 0 between the checks. In that case we hit the second loop which was only supposed to be for when a new file is created. The call to GetFullPathName passes the uninitialized temp filename buffer. If this has been initialized to zero then this call would fail but as long as the first 16 bit character in that buffer is non-zero (which is almost always the case) then GetFullPathName succees. If dwStartFlags has bit 0x40 set a call will be made to DupFileHandleToOthers which walks the list of current file streams and duplicates into each eligible process.

It wouldn’t be that useful if we just got back a file handle we already had access to, however this process will duplicate a handle from _any_ process that the process can open for PROCESS_DUP_HANDLE, which in a system service is the majority of processes (and I think BITS enables SeDebugPrivilege just to be sure). Also nothing in the process (until later) ever checks the duplicated handle is to a file so you can use this to steal process handles etc. The easiest route to stealing the current process handle using -1 won’t work directly as -1 is also used to check for invalid handles, however as the duplicated handle value is stored in the shared memory section this can be changed back to -1 during duplication.

Winning the race condition looks pretty tricky, until you realize that Init_DupFileHandle function walks a singly linked list of file streams with no bail out for infinite loops, so you can hang up the duplication process as long as needed to change the value of the path by just building a cycle into the linked list.

Fixing wise, at a minimum the double fetch should be eliminated. You should also really be checking that the handle you duplicate is a file handle.

Proof of Concept:


I’ve provided a PoC as a C# project, I’ve provided one solution for all issues, but separate projects for each bug. This PoC uses the Audio Server to create a shared section and sends the marshaled object to the BITS service. This abuses the CFileStream::InitWorker issue I highlighted above to steal a handle to the BITS service which can then be used to elevate privileges.

Note I’ve only tested this on Windows 10 1803. While I expect the underlying bugs exist on other versions offsets/behaviors I’m relying on might differ. Also note that once the BITS service crashes the DCOM activator might not realize for a while and so starting COM object will take a long time. You can get around this by manually starting the BITS service if it doesn’t auto-start. A final note, as this uses the Audio Server it might not work on VMs with the sound card disabled.

1) Compile the C# project. It will need to grab NtApiDotNet from NuGet to work.
2) Run the PoC StealHandle as a normal user.
3) You can attach a debugger, but this issue shouldn’t crash so just hit ENTER.
4) Hit enter in the PoC.

Expected Result:
The marshal fails, or the code falls back to using standard marshaled object.

Observed Result:
An arbitrary command prompt is created as SYSTEM on the current desktop.

This bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available (whichever is earlier), the bug report will become visible to the public.
 

Comment 1 by forshaw@google.com, Aug 29

Project Member
Labels: MSRC-47439

Comment 2 by forshaw@google.com, Nov 17

Project Member
Mergedinto: 1644
Status: Duplicate (was: New)
Marking as a duplicate of the master issue as only one fix was issue for all the bugs.

Comment 3 by forshaw@google.com, Nov 20

Project Member
Labels: -Restrict-View-Commit

Sign in to add a comment