Hello, a customer reported getting corrupt output when running the 'tpm_version' command. It turns out there are two issues in the code:
- NULL bytes in the TPM Vendor ID are output - Possibly undefined data is printed on stderr, since an unterminated string buffer is used. This might also lead to a crash in some circumstances Please find attached two patches that address these two issues. Regards Matthias -- Matthias Gerstner <matthias.gerst...@suse.de> Dipl.-Wirtsch.-Inf. (FH), Security Engineer https://www.suse.com/security Telefon: +49 911 740 53 290 GPG Key ID: 0x14C405C971923553 SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nuernberg)
From c927f67f36a4719bd15b8a535efb6980f1e87a6b Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerst...@suse.de>
Date: Fri, 30 Nov 2018 12:48:37 +0100
Subject: [PATCH] tpm_version: avoid outputting NULL bytes from tpmVendorID
When the vendor ID contains null bytes then '^@' characters appear in
the tpm_version output. This can confuse users and it also causes e.g.
'grep' to treat the input as binary. Example:
TPM Vendor ID: WEC\000
This change copies the vendor ID bytes over into a local string object.
This makes the code more independent of the vendor ID dimension and also
avoids NULL bytes being printed.
---
src/tpm_mgmt/tpm_version.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/tpm_mgmt/tpm_version.c b/src/tpm_mgmt/tpm_version.c
index 1019b71..78b78e8 100644
--- a/src/tpm_mgmt/tpm_version.c
+++ b/src/tpm_mgmt/tpm_version.c
@@ -133,6 +133,7 @@ int cmdVersion(const char *a_szCmd)
UINT64 offset;
TSS_RESULT uiResult;
TPM_CAP_VERSION_INFO versionInfo;
+ char vendor_id[sizeof(versionInfo.tpmVendorID)+1];
char *errbuf = NULL; // Buffer containing what was sent to stderr during getCapability.
/* Disable logging to of "Bad Mode" during this call.
@@ -169,15 +170,17 @@ int cmdVersion(const char *a_szCmd)
goto out_close;
}
+ // copy over the individual characters into a regular string.
+ // This avoids that null bytes are written to stdout.
+ snprintf ( vendor_id, sizeof(vendor_id), "%s", (const char*)versionInfo.tpmVendorID );
+
logMsg(_(" TPM 1.2 Version Info:\n"));
logMsg(_(" Chip Version: %hhu.%hhu.%hhu.%hhu\n"),
versionInfo.version.major, versionInfo.version.minor,
versionInfo.version.revMajor, versionInfo.version.revMinor);
logMsg(_(" Spec Level: %hu\n"), versionInfo.specLevel);
logMsg(_(" Errata Revision: %hhu\n"), versionInfo.errataRev);
- logMsg(_(" TPM Vendor ID: %c%c%c%c\n"),
- versionInfo.tpmVendorID[0], versionInfo.tpmVendorID[1],
- versionInfo.tpmVendorID[2], versionInfo.tpmVendorID[3]);
+ logMsg(_(" TPM Vendor ID: %s\n"), vendor_id);
if (versionInfo.vendorSpecificSize) {
logMsg(_(" Vendor Specific data: "));
--
2.18.1
From f0f30ff3e3b08751ebb8524303d80b6e94882134 Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerst...@suse.de>
Date: Fri, 30 Nov 2018 13:17:01 +0100
Subject: [PATCH] tpm_version: avoid outputting undefined data on stderr
If there was no data written to the temporary file then memsize == 1, no
data will be read from the file into the buffer and the buffer will not
be null terminated. This can cause random data to be output later on to
the original stderr like:
'#precedence ::ffff:0:0/'
or
'xl?8?'
Fix this by making sure the buffer is always zero terminated.
---
src/tpm_mgmt/tpm_version.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/tpm_mgmt/tpm_version.c b/src/tpm_mgmt/tpm_version.c
index 78b78e8..e563a8c 100644
--- a/src/tpm_mgmt/tpm_version.c
+++ b/src/tpm_mgmt/tpm_version.c
@@ -99,6 +99,9 @@ char* end_capture_stderr(int olderr)
perror("read()");
}
+ // make sure the buffer is null terminated.
+ buf[st.st_size] = '\0';
+
// Restore stderr.
errout:
if (0 > dup2(olderr, STDERR_FILENO)) {
--
2.18.1
signature.asc
Description: PGP signature
_______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel
