aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJason A. Donenfeld <Jason@zx2c4.com>2019-06-29 11:40:26 +0000
committerJason A. Donenfeld <Jason@zx2c4.com>2019-07-03 08:50:30 +0000
commit2517fb6659fc6f7504a4ed7d2aa54472c16e863f (patch)
tree499daf77ba7a91043a8aa9758e9b0a2eb93ee825
parentUse NDIS variable style for locals (diff)
downloadwintun-2517fb6659fc6f7504a4ed7d2aa54472c16e863f.tar.xz
wintun-2517fb6659fc6f7504a4ed7d2aa54472c16e863f.zip
Read userspace packet size once, before checks
Otherwise userspace can twiddle things between checks. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
-rw-r--r--wintun.c18
1 files changed, 10 insertions, 8 deletions
diff --git a/wintun.c b/wintun.c
index 06e52c7..7debdad 100644
--- a/wintun.c
+++ b/wintun.c
@@ -395,7 +395,7 @@ TunWriteIntoIrp(_Inout_ IRP *Irp, _Inout_ UCHAR *Buffer, _In_ NET_BUFFER *Nb, _I
ULONG PacketSize = NET_BUFFER_DATA_LENGTH(Nb);
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);
if (!NbData)
{
@@ -806,22 +806,24 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
}
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;
goto cleanup_nbl_queues;
}
- ULONG PacketSize = TunPacketAlign(sizeof(TUN_PACKET) + Packet->Size);
- if (BufferEnd - BufferPos < (ptrdiff_t)PacketSize)
+ ULONG AlignedPacketSize = TunPacketAlign(sizeof(TUN_PACKET) + PacketSize);
+ if (BufferEnd - BufferPos < (ptrdiff_t)AlignedPacketSize)
{
Status = STATUS_INVALID_USER_BUFFER;
goto cleanup_nbl_queues;
}
EtherTypeIndex Index;
- if (Packet->Size >= 20 && Packet->Data[0] >> 4 == 4)
+ if (PacketSize >= 20 && Packet->Data[0] >> 4 == 4)
Index = EtherTypeIndexIPv4;
- else if (Packet->Size >= 40 && Packet->Data[0] >> 4 == 6)
+ else if (PacketSize >= 40 && Packet->Data[0] >> 4 == 6)
Index = EtherTypeIndexIPv6;
else
{
@@ -830,7 +832,7 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
}
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)
{
Status = STATUS_INSUFFICIENT_RESOURCES;
@@ -845,7 +847,7 @@ TunDispatchWrite(_Inout_ TUN_CTX *Ctx, _Inout_ IRP *Irp)
TunAppendNBL(&NblQueue[Index].Head, &NblQueue[Index].Tail, Nbl);
NblQueue[Index].Count++;
NblCount++;
- BufferPos += PacketSize;
+ BufferPos += AlignedPacketSize;
}
if ((ULONG)(BufferPos - BufferStart) != Size)