Load of misaligned address in yh_util_import_wrapped()

[invd]

During fuzzing, I've found the following with -fsanitize=undefined:

runtime error: load of misaligned address 0x7b025caee261 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment

at the code position in yh_util_import_wrapped():

yubihsm-shell/lib/yubihsm.c

Line 2738 in c409285

*target_id = ntohs(*((uint16_t *) (response + 1)));

This is undefined behavior (UB) and therefore a bug.

yh_util_import_rsa_wrapped() appears to have a second instance of this pattern that is also triggered:

yubihsm-shell/lib/yubihsm.c

Line 2797 in c409285

*target_id = ntohs(*((uint16_t *) (response + 1)));

[invd]

@aveenismail @qpernil can you confirm this issue?

[aveenismail]

@invd Apologies for the very late response. There is now a PR containing a fix for this issue.

#445

[aveenismail]

I should probably explain more about #445:

The response data is now stored using pragma pack 1, so it will still be unaligned, but so are the responses to all other commands and elsewhere in the code. But since non of the other commands are triggering this error, using pragama pack 1 might be sufficient to solve you fuzzer issue.

Is this only a fuzzing issue or are you encountering this in your regular use?

[invd]

@aveenismail I encountered this while fuzzing on an x86_64 system, with different compiler versions such as clang-16 and clang-20 (snapshot). Based on the code, I suspect the undefined behavior would also be present and detectable on corresponding builds with -fsanitize=undefined during actual use of the relevant functions.

I've also updated my fuzzer code to the newest commit 3dd15f3 and can confirm the issue is still present there, as observed in at least one variant.

[aveenismail]

And it's triggered only in yh_util_import_wrapped() and yh_util_import_rsa_wrapped()? Not any other functions, say like import_key() or yh_util_sign_attestation_certificate() for example?

[qpernil]

3dd15f3 is part of #444, not #445. Would you be able to test with the changes in #445 ? Also, this looks like a false positive to me, the x86 and x86_64 architectures can handle unaligned memory accesses fine. So can most ARM architectures, such as Apple Silicon. So it is a bit puzzling to me that you get this, for two reasons: The processor can handle unaligned; plus there are lots of other unaligned accesses that should also be flagged..