tpm: Improve error handling and check returned lengths.

* tpm2d/command.c (cmd_pkdecrypt): Handle unknown algo.  Also slightly
rework error handling.
* tpm2d/tpm2.c (sexp_to_tpm2_public_ecc): Check length before checking
for 0x04.  Rework error handling.
(tpm2_ObjectPublic_GetName): Check the return value of
TSS_GetDigestSize before use.  Erro handling rework.
(tpm2_SensitiveToDuplicate): Ditto.
(tpm2_import_key): Ditto.
* tpm2d/intel-tss.h (TSS_Hash_Generate): Check passed length for
negative values.  Check return value of TSS_GetDigestSize.  Use
dedicated 16 bit length variable.
--

These are reworked and improved fixes as reported in
GnuPG-bug-id: 7129

3 files changed, 109 insertions, 65 deletions

diff --git a/tpm2d/command.c b/tpm2d/command.c
index 6f8cb5506..8f69a5e11 100644
--- a/tpm2d/command.c
+++ b/tpm2d/command.c
@@ -291,12 +291,12 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line)
 {
   ctrl_t ctrl = assuan_get_pointer (ctx);
   int rc;
-  unsigned char *shadow_info;
+  unsigned char *shadow_info = NULL;
   size_t len;
   TSS_CONTEXT *tssc;
   TPM_HANDLE key;
   TPMI_ALG_PUBLIC type;
-  unsigned char *crypto;
+  unsigned char *crypto = NULL;
   size_t cryptolen;
   char *buf;
   size_t buflen;
@@ -313,7 +313,7 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line)
 
   rc = assuan_inquire (ctx, "EXTRA", &crypto, &cryptolen, MAXLEN_KEYDATA);
   if (rc)
-    goto out_freeshadow;
+    goto out;
 
   rc = tpm2_start (&tssc);
   if (rc)
@@ -329,6 +329,11 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line)
   else if (type == TPM_ALG_ECC)
     rc = tpm2_ecc_decrypt (ctrl, tssc, key, pin_cb, crypto,
 			   cryptolen, &buf, &buflen);
+  else
+    {
+      rc = GPG_ERR_PUBKEY_ALGO;
+      goto end_out;
+    }
 
   tpm2_flush_handle (tssc, key);
 
@@ -343,7 +348,6 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line)
 
  out:
   xfree (crypto);
- out_freeshadow:
   xfree (shadow_info);
 
   return rc;
diff --git a/tpm2d/intel-tss.h b/tpm2d/intel-tss.h
index 1649cca05..da085fac7 100644
--- a/tpm2d/intel-tss.h
+++ b/tpm2d/intel-tss.h
@@ -344,7 +344,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...)
   int length;
   uint8_t *buffer;
   int algo;
-  gcry_md_hd_t md;
+  gcry_md_hd_t md = NULL;
   va_list ap;
 
   va_start(ap, digest);
@@ -353,7 +353,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...)
   if (rc)
     {
       log_error ("TSS_Hash_Generate: Unknown hash %d\n", digest->hashAlg);
-      goto out;
+      goto leave;
     }
 
   rc = gcry_md_open (&md, algo, 0);
@@ -362,7 +362,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...)
       log_error ("TSS_Hash_Generate: EVP_MD_CTX_create failed: %s\n",
                  gpg_strerror (rc));
       rc = TPM_RC_FAILURE;
-      goto out;
+      goto leave;
     }
 
   rc = TPM_RC_FAILURE;
@@ -374,19 +374,24 @@ TSS_Hash_Generate(TPMT_HA *digest, ...)
 	break;
       if (length < 0)
 	{
-	  log_error ("TSS_Hash_Generate: Length is negative\n");
-	  goto out_free;
+	  log_error ("%s: Length is negative\n", "TSS_Hash_Generate");
+	  goto leave;
 	}
       if (length != 0)
 	gcry_md_write (md, buffer, length);
     }
 
-  memcpy (&digest->digest, gcry_md_read (md, algo),
-	  TSS_GetDigestSize(digest->hashAlg));
+  length = TSS_GetDigestSize(digest->hashAlg);
+  if (length < 0)
+    {
+      log_error ("%s: Length is negative\n", "TSS_GetDigestSize");
+      goto leave;
+    }
+  memcpy (&digest->digest, gcry_md_read (md, algo), length);
   rc = TPM_RC_SUCCESS;
- out_free:
+
+ leave:
   gcry_md_close (md);
- out:
   va_end(ap);
   return rc;
 }
diff --git a/tpm2d/tpm2.c b/tpm2d/tpm2.c
index d0b32ed35..2a49bf99b 100644
--- a/tpm2d/tpm2.c
+++ b/tpm2d/tpm2.c
@@ -446,46 +446,55 @@ static int
 sexp_to_tpm2_public_ecc (TPMT_PUBLIC *p, gcry_sexp_t key)
 {
   const char *q;
-  gcry_sexp_t l;
-  int rc = GPG_ERR_BAD_PUBKEY;
+  gcry_sexp_t l = NULL;
+  int rc;
   size_t len;
   TPMI_ECC_CURVE curve;
-  char *curve_name;
+  char *curve_name = NULL;
 
   l = gcry_sexp_find_token (key, "curve", 0);
   if (!l)
-    return rc;
+    {
+      rc = GPG_ERR_NO_PUBKEY;
+      goto leave;
+    }
   curve_name = gcry_sexp_nth_string (l, 1);
   if (!curve_name)
-    goto out;
+    {
+      rc = GPG_ERR_INV_CURVE;
+      goto leave;
+    }
   rc = tpm2_ecc_curve (curve_name, &curve);
-  gcry_free (curve_name);
   if (rc)
-    goto out;
-  gcry_sexp_release (l);
+    goto leave;
 
+  gcry_sexp_release (l);
   l = gcry_sexp_find_token (key, "q", 0);
   if (!l)
-    return rc;
+    {
+      rc = GPG_ERR_NO_PUBKEY;
+      goto leave;
+    }
   q = gcry_sexp_nth_data (l, 1, &len);
   /* This is a point representation, the first byte tells you what
    * type.  The only format we understand is uncompressed (0x04)
    * which has layout 0x04 | x | y */
-  if (q[0] != 0x04)
+  if (!q || len < 2 || q[0] != 0x04)
     {
-      log_error ("Point format for q is not uncompressed\n");
-      goto out;
+      log_error ("tss: point format for q is not uncompressed\n");
+      rc = GPG_ERR_BAD_PUBKEY;
+      goto leave;
     }
   q++;
   len--;
   /* now should have to equal sized big endian point numbers */
   if ((len & 0x01) == 1)
     {
-      log_error ("Point format for q has incorrect length\n");
-      goto out;
+      log_error ("tss: point format for q has incorrect length\n");
+      rc = GPG_ERR_BAD_PUBKEY;
+      goto leave;
     }
-
-  len >>= 1;
+  len >>= 1; /* Compute length of one coordinate.  */
 
   p->type = TPM_ALG_ECC;
   p->nameAlg = TPM_ALG_SHA256;
@@ -502,7 +511,9 @@ sexp_to_tpm2_public_ecc (TPMT_PUBLIC *p, gcry_sexp_t key)
   VAL_2B (p->unique.ecc.x, size) = len;
   memcpy (VAL_2B (p->unique.ecc.y, buffer), q + len, len);
   VAL_2B (p->unique.ecc.y, size) = len;
- out:
+
+ leave:
+  gcry_free (curve_name);
   gcry_sexp_release (l);
   return rc;
 }
@@ -637,36 +648,42 @@ tpm2_ObjectPublic_GetName (NAME_2B *name,
   uint16_t written = 0;
   TPMT_HA digest;
   uint32_t sizeInBytes;
+  INT32 size = MAX_RESPONSE_SIZE;
   uint8_t buffer[MAX_RESPONSE_SIZE];
+  uint8_t *buffer1 = buffer;
+  TPMI_ALG_HASH nameAlgNbo;
+  int length;
 
   /* marshal the TPMT_PUBLIC */
-  if (rc == 0)
-    {
-      INT32 size = MAX_RESPONSE_SIZE;
-      uint8_t *buffer1 = buffer;
-      rc = TSS_TPMT_PUBLIC_Marshal (tpmtPublic, &written, &buffer1, &size);
-    }
+  rc = TSS_TPMT_PUBLIC_Marshal (tpmtPublic, &written, &buffer1, &size);
+  if (rc)
+    goto leave;
+
   /* hash the public area */
-  if (rc == 0)
-    {
-      sizeInBytes = TSS_GetDigestSize (tpmtPublic->nameAlg);
-      digest.hashAlg = tpmtPublic->nameAlg;       /* Name digest algorithm */
-      /* generate the TPMT_HA */
-      rc = TSS_Hash_Generate (&digest, written, buffer, 0, NULL);
-    }
-  if (rc == 0)
+  length = TSS_GetDigestSize (tpmtPublic->nameAlg);
+  if (length < 0)
     {
-      TPMI_ALG_HASH nameAlgNbo;
-
-      /* copy the digest */
-      memcpy (name->name + sizeof (TPMI_ALG_HASH),
-	      (uint8_t *)&digest.digest, sizeInBytes);
-      /* copy the hash algorithm */
-      nameAlgNbo = htons (tpmtPublic->nameAlg);
-      memcpy (name->name, (uint8_t *)&nameAlgNbo, sizeof (TPMI_ALG_HASH));
-      /* set the size */
-      name->size = sizeInBytes + sizeof (TPMI_ALG_HASH);
+      rc = TPM_RC_VALUE;
+      goto leave;
     }
+  sizeInBytes = length;
+  digest.hashAlg = tpmtPublic->nameAlg;       /* Name digest algorithm */
+
+  /* generate the TPMT_HA */
+  rc = TSS_Hash_Generate (&digest, written, buffer, 0, NULL);
+  if (rc)
+    goto leave;
+
+  /* copy the digest */
+  memcpy (name->name + sizeof (TPMI_ALG_HASH),
+          (uint8_t *)&digest.digest, sizeInBytes);
+  /* copy the hash algorithm */
+  nameAlgNbo = htons (tpmtPublic->nameAlg);
+  memcpy (name->name, (uint8_t *)&nameAlgNbo, sizeof (TPMI_ALG_HASH));
+  /* set the size */
+  name->size = sizeInBytes + sizeof (TPMI_ALG_HASH);
+
+ leave:
   return rc;
 }
 
@@ -694,7 +711,7 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s,
       && symdef->mode.aes == TPM_ALG_CFB)
     {
       TPMT_HA hash;
-      const int hlen = TSS_GetDigestSize (nalg);
+      int hlen;
       BYTE *digest;
       BYTE *s2b;
       int32_t size;
@@ -706,6 +723,14 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s,
        * the AES routines alter the passed in iv */
       memset (null_iv, 0, sizeof (null_iv));
 
+      hlen = TSS_GetDigestSize (nalg);
+      if (hlen < 0)
+        {
+          log_error ("%s: unknown symmetric algo id %d\n",
+                     "TSS_GetDigestSize", (int)nalg);
+          return TPM_RC_SYMMETRIC;
+        }
+
       /* reserve space for hash before the encrypted sensitive */
       digest = buf;
       bsize = sizeof (uint16_t /* TPM2B.size */) + hlen;
@@ -765,7 +790,7 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s,
     }
   else
     {
-      log_error ("Unknown symmetric algorithm\n");
+      log_error ("tss: Unknown symmetric algorithm\n");
       return TPM_RC_SYMMETRIC;
     }
 
@@ -795,7 +820,9 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc,
   TPM_RC rc;
 
   uint32_t size;
-  uint16_t len;
+  uint16_t u16len;
+  size_t len;
+  int dlen;
   BYTE *buffer;
   int ret;
   char *passphrase;
@@ -817,14 +844,22 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc,
 
   /* add an authorization password to the key which the TPM will check */
 
-  ret = pin_cb (ctrl,  _("Please enter the TPM Authorization passphrase for the key."), &passphrase);
+  ret = pin_cb (ctrl,
+           _("Please enter the TPM Authorization passphrase for the key."),
+                &passphrase);
   if (ret)
     return ret;
   len = strlen(passphrase);
-  if (len > TSS_GetDigestSize(objectPublic.publicArea.nameAlg))
+  dlen = TSS_GetDigestSize(objectPublic.publicArea.nameAlg);
+  if (dlen < 0)
+    {
+      log_error ("%s: error getting digest size\n", "TSS_GetDigestSize");
+      return GPG_ERR_DIGEST_ALGO;
+    }
+  if (len > dlen)
     {
-      len = TSS_GetDigestSize(objectPublic.publicArea.nameAlg);
-      log_error ("Truncating Passphrase to TPM allowed %d\n", len);
+      len = dlen;
+      log_info ("tss: truncating Passphrase to TPM allowed size of %zu\n", len);
     }
   VAL_2B (s.authValue, size) = len;
   memcpy (VAL_2B (s.authValue, buffer), passphrase, len);
@@ -885,16 +920,16 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc,
 
   size = sizeof (pub);
   buffer = pub;
-  len = 0;
+  u16len = 0;
   TSS_TPM2B_PUBLIC_Marshal (&objectPublic,
-                            &len, &buffer, &size);
+                            &u16len, &buffer, &size);
   pub_len = len;
 
   size = sizeof (priv);
   buffer = priv;
-  len = 0;
+  u16len = 0;
   TSS_TPM2B_PRIVATE_Marshal ((TPM2B_PRIVATE *)&outPrivate,
-			     &len, &buffer, &size);
+			     &u16len, &buffer, &size);
   priv_len = len;
 
   *shadow_info = make_tpm2_shadow_info (parent, pub, pub_len,