Coverity: Dereference of potentially null field

[philiptaff]

Describe the bug
In atcah_mac(), param->sn is only checked for null if MAC_MODE_INCLUDE_SN is set in param->mode. But param->sn is always copied to include_data.sn, which is then passed to atcah_include_data() where the pointer is dereferenced (*param->p_temp++ = param->sn[8];) regardless of MAC_MODE_INCLUDE_SN.

To Reproduce
Steps to reproduce the behavior:

1. Call atcah_mac() with param->sn = NULL and MAC_MODE_INCLUDE_SN != (param->mode & MAC_MODE_INCLUDE_SN).
2. In line 482, include_data.sn is set to null.
3. In line 493, the null check is skipped.
4. In line 546, atcah_include_data() is called, passing include_data with include_data.sn == NULL
5. In line 64, the null pointer is dereferenced: *param->p_temp++ = param->sn[8];

Expected behavior
Should param->sn always be checked for null in atcah_mac(), regardless of MAC_MODE_INCLUDE_SN? Or in atcah_include_data() should the use of param->sn be gated by MAC_MODE_INCLUDE_SN?

Additional context
This was found in a Coverity scan.

[Srinivas-E]

Hi @philiptaff ,
Thank you for sharing your feedback. We will review and make the corresponding amendements with our next CAL release. Will keep you posted once we start working on this.

Additional info: Should param->sn always be checked for null in atcah_mac(), regardless of MAC_MODE_INCLUDE_SN?
Per the CryptoAuth device working (ATECC508A/608A), the MAC command message input is always 88 bytes. The serial number bytes SN8, SN0:1 are always included in the MAC computation (positions 8 and 10 in the message structure). The MAC_MODE_INCLUDE_SN flag only controls whether SN4:7 and SN2:3 are used or replaced with zeros (positions 9 and 11).
This implies sn is always required by atcah_include_data() regardless of the MAC_MODE_INCLUDE_SN flag.