tpm2-tss: left shit of TPM2_HT causing UBSAN issues

[williamcroberts]

UBSAN reports this:

[TASK START] Wed Jan 25 12:22:39 AM UTC 2023
[    8.862415] testsuite-70.sh[488]: ../build/src/shared/tpm2-util.c:438:25: runtime error: left shift of 129 by 24 places cannot be represented in type 'int'
[    9.310999] testsuite-70.sh[488]:     #0 0x7f88d86ee152 in tpm2_get_srk /systemd-meson-build/../build/src/shared/tpm2-util.c:438:25
[    9.310999] testsuite-70.sh[488]:     #1 0x7f88d86ee152 in tpm2_make_primary /systemd-meson-build/../build/src/shared/tpm2-util.c:522:22
[    9.310999] testsuite-70.sh[488]:     #2 0x7f88d86eb6ef in tpm2_seal /systemd-meson-build/../build/src/shared/tpm2-util.c:1649:13
[    9.310999] testsuite-70.sh[488]:     #3 0x55dccda071f1 in enroll_tpm2 /systemd-meson-build/../build/src/cryptenroll/cryptenroll-tpm2.c:213:13
[    9.310999] testsuite-70.sh[488]:     #4 0x55dccda02692 in run /systemd-meson-build/../build/src/cryptenroll/cryptenroll.c:667:24
[    9.310999] testsuite-70.sh[488]:     #5 0x55dccda02692 in main /systemd-meson-build/../build/src/cryptenroll/cryptenroll.c:692:1
[    9.310999] testsuite-70.sh[488]:     #6 0x7f88d770928f  (/usr/lib/libc.so.6+0x2328f) (BuildId: 768945cdf5e5796c2ab39f38ed160748fd94d12e)
[    9.310999] testsuite-70.sh[488]:     #7 0x7f88d7709349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) (BuildId: 768945cdf5e5796c2ab39f38ed160748fd94d12e)
[    9.310999] testsuite-70.sh[488]:     #8 0x55dccd9f79a4 in _start (/usr/bin/systemd-cryptenroll+0xd9a4) (BuildId: 6f5280c50c252f04bcd2e52642beeda1f9939bf5)
[    9.310999] testsuite-70.sh[488]: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../build/src/shared/tpm2-util.c:438:25 in

On code like this:

        TSS2_RC rc = sym_Esys_GetCapability(c,
                        ESYS_TR_NONE,
                        ESYS_TR_NONE,
                        ESYS_TR_NONE,
                        TPM2_CAP_HANDLES,
                        TPM2_PERSISTENT_FIRST,
                        TPM2_MAX_CAP_HANDLES,
                        &more_data,
                        &cap_data);

I was able to reproduce this with a trivial C program:

#include <stdint.h>
#include <stdio.h>

int main(int argc, char **argv) {

        printf("%u\n", ((uint8_t)0x81) << (uint32_t)24);

        return 0;
}

Compiled like so:

clang -fsanitize=undefined -o go a.c
./go
a.c:7:33: runtime error: left shift of 129 by 24 places cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior a.c:7:33 in 
2164260864

However, this works:

#include <stdint.h>
#include <stdio.h>

int main(int argc, char **argv) {

        printf("%u\n", ((uint32_t)0x81) << (uint32_t)24);

        return 0;
}

We'll need to promote that type when used in the left shifts.

[williamcroberts]

@AndreasFuchsTPM this is a bug in the spec causing ubsan to trigger

[williamcroberts]

@JuergenReppSIT can you bring this up to @AndreasFuchsTPM , this is a bug that will require a spec change

[AndreasFuchsTPM]

So I guess the bug is here: https://github.com/tpm2-software/tpm2-tss/blob/702a8715f761e0e6127fd608b58bfe15fb2fa8dd/include/tss2/tss2_tpm2_types.h#L660-L666 https://github.com/tpm2-software/tpm2-tss/blob/702a8715f761e0e6127fd608b58bfe15fb2fa8dd/include/tss2/tss2_tpm2_types.h#L686-L686

We have to upcast to TPM2_HR before the shift since the TPM2HT* defines are all uint8_t.

[williamcroberts]

Yeah exactly