tpm2-tss/0001-tss2_rc-ensure-layer-number-is-in-bounds.patch

From 79f62668a31a2da938f83d534a49ad7f9bc144ca Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Thu, 19 Jan 2023 11:53:06 -0600
Subject: [PATCH] tss2_rc: ensure layer number is in bounds

The layer handler array was defined as 255, the max number of uint8,
which is the size of the layer field, however valid values are 0-255
allowing for 256 possibilities and thus the array was off by one and
needed to be sized to 256 entries. Update the size and add tests.

Note: previous implementations incorrectly dropped bits on unknown error
output, ie TSS2_RC of 0xFFFFFF should yeild a string of 255:0xFFFFFF,
but earlier implementations returned 255:0xFFFF, dropping the middle
bits, this patch fixes that.

Fixes: CVE-2023-22745

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
 src/tss2-rc/tss2_rc.c    | 31 +++++++++++++++++++++----------
 test/unit/test_tss2_rc.c | 21 ++++++++++++++++++++-
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/tss2-rc/tss2_rc.c b/src/tss2-rc/tss2_rc.c
index 93743048..0a64958f 100644
--- a/src/tss2-rc/tss2_rc.c
+++ b/src/tss2-rc/tss2_rc.c
@@ -1,5 +1,8 @@
 /* SPDX-License-Identifier: BSD-2-Clause */
-
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <assert.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -777,7 +780,7 @@ sys_err_handler (TSS2_RC rc)
 static struct {
     char name[TSS2_ERR_LAYER_NAME_MAX];
     TSS2_RC_HANDLER handler;
-} layer_handler[TPM2_ERROR_TSS2_RC_LAYER_COUNT] = {
+} layer_handler[TPM2_ERROR_TSS2_RC_LAYER_COUNT + 1] = {
     ADD_HANDLER("tpm" , tpm2_ehandler),
     ADD_NULL_HANDLER,                       /* layer 1  is unused */
     ADD_NULL_HANDLER,                       /* layer 2  is unused */
@@ -812,7 +815,7 @@ unknown_layer_handler(TSS2_RC rc)
     static __thread char buf[32];
 
     clearbuf(buf);
-    catbuf(buf, "0x%X", tpm2_error_get(rc));
+    catbuf(buf, "0x%X", rc);
 
     return buf;
 }
@@ -909,19 +912,27 @@ Tss2_RC_Decode(TSS2_RC rc)
         catbuf(buf, "%u:", layer);
     }
 
-    handler = !handler ? unknown_layer_handler : handler;
-
     /*
      * Handlers only need the error bits. This way they don't
      * need to concern themselves with masking off the layer
      * bits or anything else.
      */
-    UINT16 err_bits = tpm2_error_get(rc);
-    const char *e = err_bits ? handler(err_bits) : "success";
-    if (e) {
-        catbuf(buf, "%s", e);
+    if (handler) {
+        UINT16 err_bits = tpm2_error_get(rc);
+        const char *e = err_bits ? handler(err_bits) : "success";
+        if (e) {
+            catbuf(buf, "%s", e);
+        } else {
+            catbuf(buf, "0x%X", err_bits);
+        }
     } else {
-        catbuf(buf, "0x%X", err_bits);
+        /*
+         * we don't want to drop any bits if we don't know what to do with it
+         * so drop the layer byte since we we already have that.
+         */
+        const char *e = unknown_layer_handler(rc >> 8);
+        assert(e);
+        catbuf(buf, "%s", e);
     }
 
     return buf;
diff --git a/test/unit/test_tss2_rc.c b/test/unit/test_tss2_rc.c
index 1c8d66c9..9369beda 100644
--- a/test/unit/test_tss2_rc.c
+++ b/test/unit/test_tss2_rc.c
@@ -198,7 +198,7 @@ test_custom_handler(void **state)
      * Test an unknown layer
      */
     e = Tss2_RC_Decode(rc);
-    assert_string_equal(e, "1:0x2A");
+    assert_string_equal(e, "1:0x100");
 }
 
 static void
@@ -281,6 +281,23 @@ test_tcti(void **state)
     assert_string_equal(e, "tcti:Fails to connect to next lower layer");
 }
 
+static void
+test_all_FFs(void **state)
+{
+    (void) state;
+
+    const char *e = Tss2_RC_Decode(0xFFFFFFFF);
+    assert_string_equal(e, "255:0xFFFFFF");
+}
+
+static void
+test_all_FFs_set_handler(void **state)
+{
+    (void) state;
+    Tss2_RC_SetHandler(0xFF, "garbage", custom_err_handler);
+    Tss2_RC_SetHandler(0xFF, NULL, NULL);
+}
+
 /* link required symbol, but tpm2_tool.c declares it AND main, which
  * we have a main below for cmocka tests.
  */
@@ -312,6 +329,8 @@ main(int argc, char* argv[])
             cmocka_unit_test(test_esys),
             cmocka_unit_test(test_mu),
             cmocka_unit_test(test_tcti),
+            cmocka_unit_test(test_all_FFs),
+            cmocka_unit_test(test_all_FFs_set_handler)
     };
 
     return cmocka_run_group_tests(tests, NULL, NULL);
-- 
2.40.1