* [Patch 1/2] MdeModulePkg: Check for the max DHCP packet length before use it.
2016-11-16 5:38 [Patch 0/2] Check-for-the-max-DHCP-packet-length Fu Siyuan
@ 2016-11-16 5:38 ` Fu Siyuan
2016-11-16 5:38 ` [Patch 2/2] NetworkPkg: " Fu Siyuan
2016-11-18 8:10 ` [Patch 0/2] Check-for-the-max-DHCP-packet-length Wu, Jiaxin
2 siblings, 0 replies; 4+ messages in thread
From: Fu Siyuan @ 2016-11-16 5:38 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Wu Jiaxin
This patch updates the PXE 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>
---
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c | 23 ++++++++++++++++++++++
.../Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h | 2 +-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
index eac955c..f03176b 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.c
@@ -912,6 +912,14 @@ PxeBcDhcpCallBack (
case Dhcp4SendDiscover:
case Dhcp4SendRequest:
+ if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_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
@@ -942,6 +950,13 @@ PxeBcDhcpCallBack (
case Dhcp4RcvdOffer:
Status = EFI_NOT_READY;
+ if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
+ //
+ // Ignore the incoming Offers which exceed the maximum length.
+ //
+ break;
+ }
+
if (Private->NumOffers < PXEBC_MAX_OFFER_NUM) {
//
// Cache the dhcp offers in Private->Dhcp4Offers[]
@@ -967,6 +982,14 @@ PxeBcDhcpCallBack (
break;
case Dhcp4RcvdAck:
+ if (Packet->Length > PXEBC_DHCP4_MAX_PACKET_SIZE) {
+ //
+ // Abort the DHCP if the ACK packet exceeds the maximum length.
+ //
+ Status = EFI_ABORTED;
+ break;
+ }
+
//
// Cache Ack
//
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
index abdf05d..614ea75 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcDhcp.h
@@ -18,7 +18,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define PXEBC_DHCP4_MAX_OPTION_NUM 16
#define PXEBC_DHCP4_MAX_OPTION_SIZE 312
-#define PXEBC_DHCP4_MAX_PACKET_SIZE 1472
+#define PXEBC_DHCP4_MAX_PACKET_SIZE (sizeof (EFI_PXE_BASE_CODE_PACKET))
#define PXEBC_DHCP4_S_PORT 67
#define PXEBC_DHCP4_C_PORT 68
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Patch 2/2] NetworkPkg: Check for the max DHCP packet length before use it.
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
2016-11-18 8:10 ` [Patch 0/2] Check-for-the-max-DHCP-packet-length Wu, Jiaxin
2 siblings, 0 replies; 4+ messages in thread
From: Fu Siyuan @ 2016-11-16 5:38 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Wu Jiaxin
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
^ permalink raw reply related [flat|nested] 4+ messages in thread