[Winpcap-bugs] multiple security issues in WinPCap

Sebastian Gottschalk seppig_relay at gmx.de
Thu May 8 04:59:45 GMT 2008


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

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.

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.

Greetings,
Sebastian Gottschalk


More information about the Winpcap-bugs mailing list