[Winpcap-bugs] multiple security issues in WinPCap

[Gianluca Varenni]

----- Original Message ----- 
From: "Sebastian Gottschalk" <seppig_relay at gmx.de>
To: <winpcap-bugs at winpcap.org>
Sent: Wednesday, May 07, 2008 9:59 PM
Subject: [Winpcap-bugs] multiple security issues in WinPCap


> Hello there.
>
> I found the following security issues:
>
> 1. NPF_Read fails to validate buffer size if you're in statistics mode and 
> the buffer isn't yet sufficiently filled.
>
> if( Occupation <= Open->MinToCopy*NCpu || Open->mode & MODE_DUMP )
> [...]
> if(Open->mode & MODE_STAT)
> [...]
> header=(struct bpf_hdr*)CurrBuff;
> GET_TIME(&header->bh_tstamp,&G_Start_Time); <== BANG

Fixed.

>
> 2. BIOCSWRITEREP uses method IN_DIRECT, but doesn't validate if the buffer 
> is mapped and accessible, and doesn't lock it either. As this function 
> reads from the buffer, but the method can't be changed (since these IOCTLs 
> are already occupied by other functions), one must resort to the typical 
> mnemonic:
>
> __try {
> if ((Irp->RequestorMode == UserMode) || FlagOn(IrpSp->Flags,
> SL_FORCE_ACCESS_CHECK))
> ProbeForRead(Irp->AssociatedIrp.SystemBuffer,
> sizeof(UINT),TYPE_ALIGNMENT(UINT));
>
> Open->Nwrites = *((PULONG)Irp->AssociatedIrp.SystemBuffer);
> SET_RESULT_SUCCESS(0);
>
> } __except((GetExceptionCode() == STATUS_ACCESS_VIOLATION) ||
> (GetExceptionCode() == STATUS_DATATYPE_MISALIGNMENT) ? 
> EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
>
> SET_FAILURE_UNSUCCESSFUL();
>
> }
>
> I guess the meaning is pretty clear: Usermode buffer must be validated, 
> and kernelmode buffer must also be validated is the SL_FORCE_ACCESS_CHECK 
> flag is set (though kernel mode dumping is currently deactivated). 
> ProbeForRead tests for read access and correct alignment, and all further 
> access to the buffer must be enclosed within the try-except statement. The 
> ProbeForXXX macros and the buffer access can only lead to the two 
> exception codes, all other exceptions must originate from functions 
> outside the scope of the driver and must be propagated as such.

This is not needed. In the case of METHOD_IN_DIRECT, the OS guarantees that 
the buffer in Irp->AssociatedIrp.SystemBuffer is readable (this is done by 
the I/O manager). You just need to check that the buffer is long enough.

>
> 3. This is also the way how the buffer validation in BIOCGSTATS can be 
> made less error-prone (potentially locking a large number of pages) and 
> also more performant (only locking when necessary, no MDL handling etc.). 
> Though please add the ProbeForWrite macro as well.
>

I fixed the code for BIOCGSTATS: I allocate an Mdl for the amount of memory 
used by the statistics, only.

Have a nice day
GV


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