Hesham/wintun
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Read userspace packet size once, before checks

Browse Source 
Otherwise userspace can twiddle things between checks.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This commit is contained in:
Jason A. Donenfeld 2019-06-29 11:40:26 +00:00
parent dbf234926d
commit 2517fb6659
1 changed files with 10 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
wintun.c
View File

@ -395,7 +395,7 @@ TunWriteIntoIrp(_Inout_ IRP *Irp, _Inout_ UCHAR *Buffer, _In_ NET_BUFFER *Nb, _I
ULONG PacketSize = NET_BUFFER_DATA_LENGTH(Nb); ULONG PacketSize = NET_BUFFER_DATA_LENGTH(Nb);
TUN_PACKET *Packet = (TUN_PACKET *)(Buffer + Irp->IoStatus.Information); TUN_PACKET *Packet = (TUN_PACKET *)(Buffer + Irp->IoStatus.Information);
Packet->Size = PacketSize; /* We shouldn't trust Packet->Size directly for reading, because the usre controls it. */ Packet->Size = PacketSize; /* We shouldn't trust Packet->Size directly for reading, because the user controls it. */
void *NbData = NdisGetDataBuffer(Nb, PacketSize, Packet->Data, 1, 0); void *NbData = NdisGetDataBuffer(Nb, PacketSize, Packet->Data, 1, 0);
if (!NbData) if (!NbData)
{ {
@ -806,22 +806,24 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
} }
TUN_PACKET *Packet = (TUN_PACKET *)BufferPos; TUN_PACKET *Packet = (TUN_PACKET *)BufferPos;
if (Packet->Size > TUN_EXCH_MAX_IP_PACKET_SIZE) ULONG PacketSize = *(volatile ULONG *)&Packet->Size; /* Packet->Size is controlled by userspace, so don't trust it. */
if (PacketSize > TUN_EXCH_MAX_IP_PACKET_SIZE)
{ {
Status = STATUS_INVALID_USER_BUFFER; Status = STATUS_INVALID_USER_BUFFER;
goto cleanup_nbl_queues; goto cleanup_nbl_queues;
} }
ULONG PacketSize = TunPacketAlign(sizeof(TUN_PACKET) + Packet->Size); ULONG AlignedPacketSize = TunPacketAlign(sizeof(TUN_PACKET) + PacketSize);
if (BufferEnd - BufferPos < (ptrdiff_t)PacketSize) if (BufferEnd - BufferPos < (ptrdiff_t)AlignedPacketSize)
{ {
Status = STATUS_INVALID_USER_BUFFER; Status = STATUS_INVALID_USER_BUFFER;
goto cleanup_nbl_queues; goto cleanup_nbl_queues;
} }
EtherTypeIndex Index; EtherTypeIndex Index;
if (Packet->Size >= 20 && Packet->Data[0] >> 4 == 4) if (PacketSize >= 20 && Packet->Data[0] >> 4 == 4)
Index = EtherTypeIndexIPv4; Index = EtherTypeIndexIPv4;
else if (Packet->Size >= 40 && Packet->Data[0] >> 4 == 6) else if (PacketSize >= 40 && Packet->Data[0] >> 4 == 6)
Index = EtherTypeIndexIPv6; Index = EtherTypeIndexIPv6;
else else
{ {
@ -830,7 +832,7 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
} }
NET_BUFFER_LIST *Nbl = NdisAllocateNetBufferAndNetBufferList( NET_BUFFER_LIST *Nbl = NdisAllocateNetBufferAndNetBufferList(
Ctx->NBLPool, 0, 0, UserBuffer->Mdl, (ULONG)(Packet->Data - BufferStart), Packet->Size); Ctx->NBLPool, 0, 0, UserBuffer->Mdl, (ULONG)(Packet->Data - BufferStart), PacketSize);
if (!Nbl) if (!Nbl)
{ {
Status = STATUS_INSUFFICIENT_RESOURCES; Status = STATUS_INSUFFICIENT_RESOURCES;
@ -845,7 +847,7 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
TunAppendNBL(&NblQueue[Index].Head, &NblQueue[Index].Tail, Nbl); TunAppendNBL(&NblQueue[Index].Head, &NblQueue[Index].Tail, Nbl);
NblQueue[Index].Count++; NblQueue[Index].Count++;
NblCount++; NblCount++;
BufferPos += PacketSize; BufferPos += AlignedPacketSize;
} }
if ((ULONG)(BufferPos - BufferStart) != Size) if ((ULONG)(BufferPos - BufferStart) != Size)