[Winpcap-bugs] Re: WinPCap bugs

[Gianluca Varenni]

----- Original Message ----- 
From: "Sebastian Gottschalk" <seppig_relay at gmx.de>
To: <winpcap-bugs at winpcap.org>
Sent: Thursday, February 28, 2008 12:01 PM
Subject: [Winpcap-bugs] Re: WinPCap bugs


> Dear Sir or Madam,
>
> I'm sorry to tell you that I've lost about a week of mails, thus I can 
> only reply to what I remember.

If the e-mail went to winpcap-bugs, it's archived here

http://www.winpcap.org/pipermail/winpcap-bugs/

I usually suggest people to never reply to me directly, and to always put 
winpcap-bugs in CC.

>
> As for your question about the hypothetical integer overflow:
> It may happen in the loop where all the Open->Buffer[i] get locked with a 
> spinlock, that is, if *Open+i*sizeof(OPEN_INSTANCE) overflows. Since 'i' 
> is limited by nCpu, this is clearly impossible to exploit.

I'm still not sure what you mean by "*Open +i*sizeof(OPEN_INSTANCE)". The 
only risk of overflow is if
i > 32 (in which case Open->Buffer[i] overwrites some byte inside 
OPEN_INSTANCE or even passed the end of the structure itself).

I think you are referring to this code

//
// acquire the locks for all the buffers
//
for (i = 0; i < NCpu ; i++)
{
#pragma prefast(suppress:8103, "There's no Spinlock leak here, as it's 
released some lines below.")
   NdisAcquireSpinLock(&Open->CpuData[i].BufferLock);
}

CpuData is declared as

//// KAFFINITY is used as a bit mask for the affinity in the system. So on 
every supported OS is big enough for all the CPUs on the system (32 bits on 
x86, 64 on x64?).
// We use its size to compute the max number of CPUs.
//
CpuPrivateData CpuData[sizeof(KAFFINITY) * 8]; ///< Pool of kernel buffer 
structures, one for each CPU.

As you can see, the declaration is done in such a way that CpuData is always 
big enough to account for all the possible CPUs on the target machine (x86 
or x64).
It's obviously possible to add another check directly in the DriverEntry, 
where NCpu is computed, like

if (NCpu > (sizeof(KAFFINITY) * 8))
{
    //
    // fail to load the driver
    //
}

but it's quite useless.

>
> For the FsContext problem, I took the wrong description: The problem might 
> be that opening the same device with different streams might share the 
> same FsContext pointer (because the device doesn't support streams), and 
> thus it may leak an OPEN_INSTANCE structure. The documentation is unclear 
> about this, but after some experimentation I found that this doesn't seem 
> to be the case.
>

I tried it myself, and it doesn't seem to be possible to open a stream over 
the NPF device. I would be *extremely* surprised if this behavior wer 
enabled by default on a device created by a driver. First of all, it's a 
feature related to file system drivers (and up to a couple years ago, the 
documentation for FS drivers, including the link you sent me, was not public 
at all). Second, most of the driver samples in the DDK/WDK (the ones not 
related to FS) make general use of the FsContext in their IRP_MJ_CREATE 
handler freely. Just to give you an example, this is snippet from the WDM 
ndisprot (NDIS 5.0) sample from WDK6000

------
NTSTATUS
NdisProtOpen(
    IN PDEVICE_OBJECT   pDeviceObject,
    IN PIRP             pIrp
    )
{
    PIO_STACK_LOCATION      pIrpSp;
    NTSTATUS                NtStatus = STATUS_SUCCESS;

    UNREFERENCED_PARAMETER(pDeviceObject);

    pIrpSp = IoGetCurrentIrpStackLocation(pIrp);
    pIrpSp->FileObject->FsContext = NULL;
------

As you can see, it clearly ignores any previous value of FsContext and 
zeroes it.

> However, I found another bug, this time in the installer: When the Network 
> Load Balancing protocol is installed, the installer writes some settings 
> to HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows 
> NT\CurrentVersion\Tracing\Microsoft\NLB. This is obviously wrong, it 
> should be the subkey NLBMPROV of the mentioned key (which typically 
> already contains the values the installer tries to write).

See my other e-mail


Have a nice day
GV

>
> Sincerely,
> Sebastian Gottschalk
> _______________________________________________
> Winpcap-bugs mailing list
> Winpcap-bugs at winpcap.org
> https://www.winpcap.org/mailman/listinfo/winpcap-bugs