From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 22 Mar 2018 11:00:55 +0000 (+0100)
Subject: NetworkPkg/HttpDxe: sanity-check the TlsCaCertificate variable before use
X-Git-Tag: edk2-stable201903~2037
X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=0fd13678a6818c1bc241b21f83a3013b17a55a25;ds=sidebyside

NetworkPkg/HttpDxe: sanity-check the TlsCaCertificate variable before use

In TlsConfigCertificate(), make sure that the set of EFI_SIGNATURE_LIST
objects that the platform stored to "TlsCaCertificate" is well-formed.

In addition, because HttpInstance->TlsConfiguration->SetData() expects
X509 certificates only, ensure that the EFI_SIGNATURE_LIST objects only
report X509 certificates, as described under EFI_CERT_X509_GUID in the
UEFI-2.7 spec.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=909
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
---

diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 938e894d9f..6c0688d130 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -75,9 +75,10 @@
 [Guids]
   gEfiTlsCaCertificateGuid                         ## SOMETIMES_CONSUMES  ## Variable:L"TlsCaCertificate"
   gEdkiiHttpTlsCipherListGuid                      ## SOMETIMES_CONSUMES  ## Variable:L"HttpTlsCipherList"
+  gEfiCertX509Guid                                 ## SOMETIMES_CONSUMES  ## GUID  # Check the cert type
 
 [Pcd]
   gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections       ## CONSUMES  
 
 [UserExtensions.TianoCore."ExtraFiles"]
-  HttpDxeExtra.uni
\ No newline at end of file
+  HttpDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index baab77225f..d658512f6d 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -384,6 +384,7 @@ TlsConfigCertificate (
   UINT32              Index;
   EFI_SIGNATURE_LIST  *CertList;
   EFI_SIGNATURE_DATA  *Cert;
+  UINTN               CertArraySizeInBytes;
   UINTN               CertCount;
   UINT32              ItemDataSize;
 
@@ -429,6 +430,70 @@ TlsConfigCertificate (
 
   ASSERT (CACert != NULL);
 
+  //
+  // Sanity check
+  //
+  Status = EFI_INVALID_PARAMETER;
+  CertCount = 0;
+  ItemDataSize = (UINT32) CACertSize;
+  while (ItemDataSize > 0) {
+    if (ItemDataSize < sizeof (EFI_SIGNATURE_LIST)) {
+      DEBUG ((DEBUG_ERROR, "%a: truncated EFI_SIGNATURE_LIST header\n",
+        __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    CertList = (EFI_SIGNATURE_LIST *) (CACert + (CACertSize - ItemDataSize));
+
+    if (CertList->SignatureListSize < sizeof (EFI_SIGNATURE_LIST)) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: SignatureListSize too small for EFI_SIGNATURE_LIST\n",
+        __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    if (CertList->SignatureListSize > ItemDataSize) {
+      DEBUG ((DEBUG_ERROR, "%a: truncated EFI_SIGNATURE_LIST body\n",
+        __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    if (!CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {
+      DEBUG ((DEBUG_ERROR, "%a: only X509 certificates are supported\n",
+        __FUNCTION__));
+      Status = EFI_UNSUPPORTED;
+      goto FreeCACert;
+    }
+
+    if (CertList->SignatureHeaderSize != 0) {
+      DEBUG ((DEBUG_ERROR, "%a: SignatureHeaderSize must be 0 for X509\n",
+        __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    if (CertList->SignatureSize < sizeof (EFI_SIGNATURE_DATA)) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: SignatureSize too small for EFI_SIGNATURE_DATA\n", __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    CertArraySizeInBytes = (CertList->SignatureListSize -
+                            sizeof (EFI_SIGNATURE_LIST));
+    if (CertArraySizeInBytes % CertList->SignatureSize != 0) {
+      DEBUG ((DEBUG_ERROR,
+        "%a: EFI_SIGNATURE_DATA array not a multiple of SignatureSize\n",
+        __FUNCTION__));
+      goto FreeCACert;
+    }
+
+    CertCount += CertArraySizeInBytes / CertList->SignatureSize;
+    ItemDataSize -= CertList->SignatureListSize;
+  }
+  if (CertCount == 0) {
+    DEBUG ((DEBUG_ERROR, "%a: no X509 certificates provided\n", __FUNCTION__));
+    goto FreeCACert;
+  }
+
   //
   // Enumerate all data and erasing the target item.
   //