Sorry, this message went up the wrong mailing list. It was destined for trousers tpm 1.2 tpm-tools. Please excuse the noise.
-- 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) On Fri, Nov 30, 2018 at 01:55:10PM +0100, Matthias Gerstner wrote: > 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 > > _______________________________________________ > tboot-devel mailing list > tboot-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tboot-devel
signature.asc
Description: PGP signature
_______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel
