public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Fu Siyuan <siyuan.fu@intel.com>
To: edk2-devel@lists.01.org
Cc: Ye Ting <ting.ye@intel.com>, Wu Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch 2/2] NetworkPkg: Check for the max DHCP packet length before use it.
Date: Wed, 16 Nov 2016 13:38:43 +0800	[thread overview]
Message-ID: <1479274723-9468-3-git-send-email-siyuan.fu@intel.com> (raw)
In-Reply-To: <1479274723-9468-1-git-send-email-siyuan.fu@intel.com>

This patch updates the PXE and HTTP boot driver to drop the input DHCP packet
if it exceed the maximum length.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |  4 +++-
 NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |  6 ++++++
 NetworkPkg/HttpBootDxe/HttpBootDhcp6.h |  4 +++-
 NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  4 ++--
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   | 34 ++++++++++++++++++++++++++++++++--
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.h   |  6 ++++--
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c   | 29 +++++++++++++++++++++++++++++
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h   |  6 ++++--
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c    | 16 ++++++++--------
 9 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.h b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.h
index 27d9498..0b2cafb 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp4.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp4.h
@@ -178,10 +178,12 @@ typedef struct {
   UINT32         Reserved;
 } HTTP_BOOT_VENDOR_OPTION;
 
+#define HTTP_CACHED_DHCP4_PACKET_MAX_SIZE  (OFFSET_OF (EFI_DHCP4_PACKET, Dhcp4) + HTTP_BOOT_DHCP4_PACKET_MAX_SIZE)
+
 typedef union {
   EFI_DHCP4_PACKET        Offer;
   EFI_DHCP4_PACKET        Ack;
-  UINT8                   Buffer[HTTP_BOOT_DHCP4_PACKET_MAX_SIZE];
+  UINT8                   Buffer[HTTP_CACHED_DHCP4_PACKET_MAX_SIZE];
 } HTTP_BOOT_DHCP4_PACKET;
 
 typedef struct {
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
index 8478642..ca84f2a 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
@@ -427,6 +427,12 @@ HttpBootDhcp6CallBack (
     
    case Dhcp6RcvdAdvertise:
      Status = EFI_NOT_READY;
+    if (Packet->Length > HTTP_BOOT_DHCP6_PACKET_MAX_SIZE) {
+      //
+      // Ignore the incoming packets which exceed the maximum length.
+      //
+      break;
+    }
      if (Private->OfferNum < HTTP_BOOT_OFFER_MAX_NUM) {
        //
        // Cache the dhcp offers to OfferBuffer[] for select later, and record
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.h b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.h
index 14d6db0..9f29898 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.h
@@ -75,10 +75,12 @@ typedef union {
   HTTP_BOOT_DHCP6_OPTION_VENDOR_CLASS   *VendorClass;
 } HTTP_BOOT_DHCP6_OPTION_ENTRY;
 
+#define HTTP_CACHED_DHCP6_PACKET_MAX_SIZE  (OFFSET_OF (EFI_DHCP6_PACKET, Dhcp6) + HTTP_BOOT_DHCP6_PACKET_MAX_SIZE)
+
 typedef union {
   EFI_DHCP6_PACKET        Offer;
   EFI_DHCP6_PACKET        Ack;
-  UINT8                   Buffer[HTTP_BOOT_DHCP6_PACKET_MAX_SIZE];
+  UINT8                   Buffer[HTTP_CACHED_DHCP6_PACKET_MAX_SIZE];
 } HTTP_BOOT_DHCP6_PACKET;
 
 typedef struct {
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index babd3e6..cf6de80 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -126,11 +126,11 @@ HttpBootStart (
   ZeroMem (Private->OfferBuffer, sizeof (Private->OfferBuffer));
   if (!Private->UsingIpv6) {
     for (Index = 0; Index < HTTP_BOOT_OFFER_MAX_NUM; Index++) {
-      Private->OfferBuffer[Index].Dhcp4.Packet.Offer.Size = HTTP_BOOT_DHCP4_PACKET_MAX_SIZE;
+      Private->OfferBuffer[Index].Dhcp4.Packet.Offer.Size = HTTP_CACHED_DHCP4_PACKET_MAX_SIZE;
     }
   } else {
     for (Index = 0; Index < HTTP_BOOT_OFFER_MAX_NUM; Index++) {
-      Private->OfferBuffer[Index].Dhcp6.Packet.Offer.Size = HTTP_BOOT_DHCP6_PACKET_MAX_SIZE;
+      Private->OfferBuffer[Index].Dhcp6.Packet.Offer.Size = HTTP_CACHED_DHCP6_PACKET_MAX_SIZE;
     }
   }
 
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
index 6566afd..44b0714 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
@@ -256,7 +256,7 @@ PxeBcBuildDhcp4Options (
     OptList[Index]->OpCode  = DHCP4_TAG_MAXMSG;
     OptList[Index]->Length  = (UINT8) sizeof (PXEBC_DHCP4_OPTION_MAX_MESG_SIZE);
     OptEnt.MaxMesgSize      = (PXEBC_DHCP4_OPTION_MAX_MESG_SIZE *) OptList[Index]->Data;
-    Value                   = NTOHS (PXEBC_DHCP4_PACKET_MAX_SIZE - 8);
+    Value                   = NTOHS (PXEBC_DHCP4_PACKET_MAX_SIZE);
     CopyMem (&OptEnt.MaxMesgSize->Size, &Value, sizeof (UINT16));
     Index++;
     OptList[Index]          = GET_NEXT_DHCP_OPTION (OptList[Index - 1]);
@@ -1183,7 +1183,7 @@ PxeBcDhcp4CallBack (
                  DHCP4_TAG_MAXMSG
                  );
   if (MaxMsgSize != NULL) {
-    Value = HTONS (PXEBC_DHCP4_PACKET_MAX_SIZE - 8);
+    Value = HTONS (PXEBC_DHCP4_PACKET_MAX_SIZE);
     CopyMem (MaxMsgSize->Data, &Value, sizeof (Value));
   }
 
@@ -1209,6 +1209,14 @@ PxeBcDhcp4CallBack (
   switch (Dhcp4Event) {
 
   case Dhcp4SendDiscover:
+    if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) {
+      //
+      // If the to be sent packet exceeds the maximum length, abort the DHCP process.
+      //
+      Status = EFI_ABORTED;
+      break;
+    }
+
     //
     // Cache the DHCPv4 discover packet to mode data directly.
     // It need to check SendGuid as well as Dhcp4SendRequest.
@@ -1216,6 +1224,14 @@ PxeBcDhcp4CallBack (
     CopyMem (&Mode->DhcpDiscover.Dhcpv4, &Packet->Dhcp4, Packet->Length);
 
   case Dhcp4SendRequest:
+    if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) {
+      //
+      // If the to be sent packet exceeds the maximum length, abort the DHCP process.
+      //
+      Status = EFI_ABORTED;
+      break;
+    }
+    
     if (Mode->SendGUID) {
       //
       // Send the system Guid instead of the MAC address as the hardware address if required.
@@ -1232,6 +1248,12 @@ PxeBcDhcp4CallBack (
 
   case Dhcp4RcvdOffer:
     Status = EFI_NOT_READY;
+    if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) {
+      //
+      // Ignore the incoming packets which exceed the maximum length.
+      //
+      break;
+    }
     if (Private->OfferNum < PXEBC_OFFER_MAX_NUM) {
       //
       // Cache the DHCPv4 offers to OfferBuffer[] for select later, and record
@@ -1256,6 +1278,14 @@ PxeBcDhcp4CallBack (
     break;
 
   case Dhcp4RcvdAck:
+    if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) {
+      //
+      // Abort the DHCP if the ACK packet exceeds the maximum length.
+      //
+	  Status = EFI_ABORTED;
+	  break;
+    }
+
     //
     // Cache the DHCPv4 ack to Private->Dhcp4Ack, but it's not the final ack in mode data
     // without verification.
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.h
index 3aabaed..27794c9 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.h
@@ -18,7 +18,7 @@
 
 #define PXEBC_DHCP4_OPTION_MAX_NUM         16
 #define PXEBC_DHCP4_OPTION_MAX_SIZE        312
-#define PXEBC_DHCP4_PACKET_MAX_SIZE        1472
+#define PXEBC_DHCP4_PACKET_MAX_SIZE        (sizeof (EFI_PXE_BASE_CODE_PACKET))
 #define PXEBC_DHCP4_S_PORT                 67
 #define PXEBC_DHCP4_C_PORT                 68
 #define PXEBC_BS_DOWNLOAD_PORT             69
@@ -263,10 +263,12 @@ typedef struct {
   UINT8                 CredTypeLen;
 } PXEBC_VENDOR_OPTION;
 
+#define PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE  (OFFSET_OF (EFI_DHCP4_PACKET, Dhcp4) + PXEBC_DHCP4_PACKET_MAX_SIZE)
+
 typedef union {
   EFI_DHCP4_PACKET        Offer;
   EFI_DHCP4_PACKET        Ack;
-  UINT8                   Buffer[PXEBC_DHCP4_PACKET_MAX_SIZE];
+  UINT8                   Buffer[PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE];
 } PXEBC_DHCP4_PACKET;
 
 typedef struct {
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
index eba8e1d..6a08e9a 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -1919,6 +1919,14 @@ PxeBcDhcp6CallBack (
   switch (Dhcp6Event) {
 
   case Dhcp6SendSolicit:
+    if (Packet->Length > PXEBC_DHCP6_PACKET_MAX_SIZE) {
+      //
+      // If the to be sent packet exceeds the maximum length, abort the DHCP process.
+      //
+      Status = EFI_ABORTED;
+      break;
+    }
+    
     //
     // Record the first Solicate msg time
     //
@@ -1934,6 +1942,12 @@ PxeBcDhcp6CallBack (
 
   case Dhcp6RcvdAdvertise:
     Status = EFI_NOT_READY;
+    if (Packet->Length > PXEBC_DHCP6_PACKET_MAX_SIZE) {
+      //
+      // Ignore the incoming packets which exceed the maximum length.
+      //
+      break;
+    }
     if (Private->OfferNum < PXEBC_OFFER_MAX_NUM) {
       //
       // Cache the dhcp offers to OfferBuffer[] for select later, and record
@@ -1944,6 +1958,14 @@ PxeBcDhcp6CallBack (
     break;
 
   case Dhcp6SendRequest:
+    if (Packet->Length > PXEBC_DHCP6_PACKET_MAX_SIZE) {
+      //
+      // If the to be sent packet exceeds the maximum length, abort the DHCP process.
+      //
+      Status = EFI_ABORTED;
+      break;
+    }
+    
     //
     // Store the request packet as seed packet for discover.
     //
@@ -1975,6 +1997,13 @@ PxeBcDhcp6CallBack (
     break;
 
   case Dhcp6RcvdReply:
+    if (Packet->Length > PXEBC_DHCP6_PACKET_MAX_SIZE) {
+      //
+      // Abort the DHCP if the Peply packet exceeds the maximum length.
+      //
+	  Status = EFI_ABORTED;
+      break;
+    }
     //
     // Cache the dhcp ack to Private->Dhcp6Ack, but it's not the final ack in mode data
     // without verification.
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
index 9493b16..39efcfa 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.h
@@ -18,7 +18,7 @@
 
 #define PXEBC_DHCP6_OPTION_MAX_NUM        16
 #define PXEBC_DHCP6_OPTION_MAX_SIZE       312
-#define PXEBC_DHCP6_PACKET_MAX_SIZE       1472
+#define PXEBC_DHCP6_PACKET_MAX_SIZE       (sizeof (EFI_PXE_BASE_CODE_PACKET))
 #define PXEBC_IP6_POLICY_MAX              0xff
 #define PXEBC_IP6_ROUTE_TABLE_TIMEOUT     10
 
@@ -101,10 +101,12 @@ typedef struct {
   UINT8                   Precedence;
 } PXEBC_DHCP6_OPTION_NODE;
 
+#define PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE  (OFFSET_OF (EFI_DHCP6_PACKET, Dhcp6) + PXEBC_DHCP6_PACKET_MAX_SIZE)
+
 typedef union {
   EFI_DHCP6_PACKET        Offer;
   EFI_DHCP6_PACKET        Ack;
-  UINT8                   Buffer[PXEBC_DHCP6_PACKET_MAX_SIZE];
+  UINT8                   Buffer[PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE];
 } PXEBC_DHCP6_PACKET;
 
 typedef struct {
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index 0552174..e24c573 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -102,12 +102,12 @@ EfiPxeBcStart (
     // PXE over IPv6 starts here, initialize the fields and list header.
     //
     Private->Ip6Policy                          = PXEBC_IP6_POLICY_MAX;
-    Private->ProxyOffer.Dhcp6.Packet.Offer.Size = PXEBC_DHCP6_PACKET_MAX_SIZE;
-    Private->DhcpAck.Dhcp6.Packet.Ack.Size      = PXEBC_DHCP6_PACKET_MAX_SIZE;
-    Private->PxeReply.Dhcp6.Packet.Ack.Size     = PXEBC_DHCP6_PACKET_MAX_SIZE;
+    Private->ProxyOffer.Dhcp6.Packet.Offer.Size = PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE;
+    Private->DhcpAck.Dhcp6.Packet.Ack.Size      = PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE;
+    Private->PxeReply.Dhcp6.Packet.Ack.Size     = PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE;
 
     for (Index = 0; Index < PXEBC_OFFER_MAX_NUM; Index++) {
-      Private->OfferBuffer[Index].Dhcp6.Packet.Offer.Size = PXEBC_DHCP6_PACKET_MAX_SIZE;
+      Private->OfferBuffer[Index].Dhcp6.Packet.Offer.Size = PXEBC_CACHED_DHCP6_PACKET_MAX_SIZE;
     }
 
     //
@@ -154,12 +154,12 @@ EfiPxeBcStart (
     //
     // PXE over IPv4 starts here, initialize the fields.
     //
-    Private->ProxyOffer.Dhcp4.Packet.Offer.Size = PXEBC_DHCP4_PACKET_MAX_SIZE;
-    Private->DhcpAck.Dhcp4.Packet.Ack.Size      = PXEBC_DHCP4_PACKET_MAX_SIZE;
-    Private->PxeReply.Dhcp4.Packet.Ack.Size     = PXEBC_DHCP4_PACKET_MAX_SIZE;
+    Private->ProxyOffer.Dhcp4.Packet.Offer.Size = PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE;
+    Private->DhcpAck.Dhcp4.Packet.Ack.Size      = PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE;
+    Private->PxeReply.Dhcp4.Packet.Ack.Size     = PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE;
 
     for (Index = 0; Index < PXEBC_OFFER_MAX_NUM; Index++) {
-      Private->OfferBuffer[Index].Dhcp4.Packet.Offer.Size = PXEBC_DHCP4_PACKET_MAX_SIZE;
+      Private->OfferBuffer[Index].Dhcp4.Packet.Offer.Size = PXEBC_CACHED_DHCP4_PACKET_MAX_SIZE;
     }
 
     PxeBcSeedDhcp4Packet (&Private->SeedPacket, Private->Udp4Read);
-- 
2.7.4.windows.1



  parent reply	other threads:[~2016-11-16  5:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16  5:38 [Patch 0/2] Check-for-the-max-DHCP-packet-length Fu Siyuan
2016-11-16  5:38 ` [Patch 1/2] MdeModulePkg: Check for the max DHCP packet length before use it Fu Siyuan
2016-11-16  5:38 ` Fu Siyuan [this message]
2016-11-18  8:10 ` [Patch 0/2] Check-for-the-max-DHCP-packet-length Wu, Jiaxin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1479274723-9468-3-git-send-email-siyuan.fu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox