Skip to content

Commit 09c75f2

Browse files
committed
Fixes for peer review.
1 parent c7ca035 commit 09c75f2

1 file changed

Lines changed: 50 additions & 24 deletions

File tree

wolfcrypt/src/port/st/stsafe.c

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
#include <wolfssl/wolfcrypt/types.h>
2727
#include <wolfssl/wolfcrypt/port/st/stsafe.h>
2828
#include <wolfssl/wolfcrypt/logging.h>
29+
#ifndef NO_ASN
30+
#include <wolfssl/wolfcrypt/asn.h>
31+
#endif
2932

3033
#ifndef STSAFE_INTERFACE_PRINTF
3134
#define STSAFE_INTERFACE_PRINTF(...) WC_DO_NOTHING
@@ -609,15 +612,30 @@ static int stsafe_sign(stsafe_slot_t slot, stsafe_curve_id_t curve_id,
609612
if (status_code == STSAFE_A_OK && signature != NULL) {
610613
/* Parse signature - format is: len(2) || R || len(2) || S */
611614
r_length = ((uint16_t)signature->Data[0] << 8) | signature->Data[1];
612-
s_length = ((uint16_t)signature->Data[2 + r_length] << 8) |
613-
signature->Data[3 + r_length];
614-
615-
/* Copy R and S to output (zero-padded) */
616-
XMEMSET(pSigRS, 0, key_sz * 2);
617-
XMEMCPY(pSigRS + (key_sz - r_length), &signature->Data[2], r_length);
618-
XMEMCPY(pSigRS + key_sz + (key_sz - s_length),
619-
&signature->Data[4 + r_length], s_length);
620-
rc = STSAFE_A_OK;
615+
616+
/* Bounds check: r_length must be valid and fit within signature buffer */
617+
if (r_length > key_sz || r_length == 0 ||
618+
(size_t)(2 + r_length + 2) > signature->Length) {
619+
rc = ASN_PARSE_E;
620+
}
621+
else {
622+
s_length = ((uint16_t)signature->Data[2 + r_length] << 8) |
623+
signature->Data[3 + r_length];
624+
625+
/* Bounds check: s_length must be valid and fit within signature buffer */
626+
if (s_length > key_sz || s_length == 0 ||
627+
(size_t)(4 + r_length + s_length) > signature->Length) {
628+
rc = ASN_PARSE_E;
629+
}
630+
else {
631+
/* Copy R and S to output (zero-padded) */
632+
XMEMSET(pSigRS, 0, key_sz * 2);
633+
XMEMCPY(pSigRS + (key_sz - r_length), &signature->Data[2], r_length);
634+
XMEMCPY(pSigRS + key_sz + (key_sz - s_length),
635+
&signature->Data[4 + r_length], s_length);
636+
rc = STSAFE_A_OK;
637+
}
638+
}
621639
}
622640
else {
623641
rc = (int)status_code;
@@ -811,22 +829,26 @@ static int stsafe_read_certificate(uint8_t** ppCert, uint32_t* pCertLen)
811829
status_code = StSafeA_Read(g_stsafe_handle, 0, 0, STSAFE_A_ALWAYS,
812830
0, 0, 4, &readBuf, STSAFE_A_NO_MAC);
813831

814-
if (status_code == STSAFE_A_OK && readBuf->Length == 4 &&
815-
readBuf->Data[0] == 0x30) {
816-
/* Parse ASN.1 length */
817-
switch (readBuf->Data[1]) {
818-
case 0x81:
819-
*pCertLen = readBuf->Data[2] + 3;
820-
break;
821-
case 0x82:
822-
*pCertLen = ((uint16_t)readBuf->Data[2] << 8) +
832+
if (status_code == STSAFE_A_OK && readBuf->Length == 4) {
833+
/* Parse ASN.1 DER certificate header */
834+
/* 0x30 = ASN_SEQUENCE | ASN_CONSTRUCTED (certificate is a SEQUENCE) */
835+
if (readBuf->Data[0] == (ASN_SEQUENCE | ASN_CONSTRUCTED)) {
836+
/* Parse ASN.1 length encoding */
837+
switch (readBuf->Data[1]) {
838+
case (ASN_LONG_LENGTH | 0x01): /* Length encoded in 1 byte */
839+
*pCertLen = readBuf->Data[2] + 3;
840+
break;
841+
case (ASN_LONG_LENGTH | 0x02): /* Length encoded in 2 bytes */
842+
*pCertLen = ((uint16_t)readBuf->Data[2] << 8) +
823843
readBuf->Data[3] + 4;
824-
break;
825-
default:
826-
if (readBuf->Data[1] < 0x81) {
827-
*pCertLen = readBuf->Data[1] + 2;
828-
}
829-
break;
844+
break;
845+
default:
846+
/* Short form: length < 128, encoded directly */
847+
if (readBuf->Data[1] < ASN_LONG_LENGTH) {
848+
*pCertLen = readBuf->Data[1] + 2;
849+
}
850+
break;
851+
}
830852
}
831853
}
832854
else {
@@ -843,6 +865,10 @@ static int stsafe_read_certificate(uint8_t** ppCert, uint32_t* pCertLen)
843865
}
844866

845867
if (rc == STSAFE_A_OK && *pCertLen > 0) {
868+
/* STSAFE-A100/A110 maximum read size is 225 bytes per command.
869+
* When CRC is supported, 2 bytes are used for CRC, leaving 223 bytes
870+
* for data. Without CRC, we can read up to 225 bytes, but use 223
871+
* for consistency and to leave room for protocol overhead. */
846872
step = 223 - (stsafe_a->CrcSupport ? 2 : 0);
847873

848874
for (i = 0; rc == STSAFE_A_OK && i < *pCertLen / step; i++) {

0 commit comments

Comments
 (0)