Skip to content

Commit 25466a9

Browse files
dgarskedgarske
authored
Merge pull request #461 from aidangarske/fix-fenrir-wolftpm
Fix security vulnerabilities identified by Fenrir scanner
2 parents 960cb90 + e50586d commit 25466a9

8 files changed

Lines changed: 245 additions & 55 deletions

File tree

‎src/tpm2.c‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,9 @@ TPM_RC TPM2_IncrementalSelfTest(IncrementalSelfTest_In* in,
851851
rc = TPM2_SendCommand(ctx, &packet);
852852
if (rc == TPM_RC_SUCCESS) {
853853
TPM2_Packet_ParseU32(&packet, &out->toDoList.count);
854+
if (out->toDoList.count > MAX_ALG_LIST_SIZE) {
855+
out->toDoList.count = MAX_ALG_LIST_SIZE;
856+
}
854857
for (i=0; i<(int)out->toDoList.count; i++) {
855858
TPM2_Packet_ParseU16(&packet, &out->toDoList.algorithms[i]);
856859
}
@@ -1163,8 +1166,16 @@ TPM_RC TPM2_PCR_Read(PCR_Read_In* in, PCR_Read_Out* out)
11631166
TPM2_Packet_ParseU32(&packet, &out->pcrUpdateCounter);
11641167
TPM2_Packet_ParsePCR(&packet, &out->pcrSelectionOut);
11651168
TPM2_Packet_ParseU32(&packet, &out->pcrValues.count);
1169+
if (out->pcrValues.count > 8) {
1170+
out->pcrValues.count = 8;
1171+
}
11661172
for (i=0; i<(int)out->pcrValues.count; i++) {
11671173
TPM2_Packet_ParseU16(&packet, &out->pcrValues.digests[i].size);
1174+
if (out->pcrValues.digests[i].size >
1175+
sizeof(out->pcrValues.digests[i].buffer)) {
1176+
out->pcrValues.digests[i].size =
1177+
sizeof(out->pcrValues.digests[i].buffer);
1178+
}
11681179
TPM2_Packet_ParseBytes(&packet,
11691180
out->pcrValues.digests[i].buffer,
11701181
out->pcrValues.digests[i].size);
@@ -2685,6 +2696,9 @@ TPM_RC TPM2_EventSequenceComplete(EventSequenceComplete_In* in,
26852696
TPM2_Packet_ParseU32(&packet, &paramSz);
26862697

26872698
TPM2_Packet_ParseU32(&packet, &out->results.count);
2699+
if (out->results.count > HASH_COUNT) {
2700+
out->results.count = HASH_COUNT;
2701+
}
26882702
for (i=0; i<(int)out->results.count; i++) {
26892703
TPM2_Packet_ParseU16(&packet,
26902704
&out->results.digests[i].hashAlg);
@@ -2876,7 +2890,7 @@ TPM_RC TPM2_GetSessionAuditDigest(GetSessionAuditDigest_In* in,
28762890
if (rc == TPM_RC_SUCCESS) {
28772891
TPM2_Packet packet;
28782892
CmdInfo_t info = {0,0,0,0};
2879-
info.inHandleCnt = 2;
2893+
info.inHandleCnt = 3;
28802894
info.flags = (CMD_FLAG_ENC2 | CMD_FLAG_DEC2 | CMD_FLAG_AUTH_USER1 |
28812895
CMD_FLAG_AUTH_USER2);
28822896

@@ -3292,7 +3306,10 @@ TPM_RC TPM2_PCR_Event(PCR_Event_In* in, PCR_Event_Out* out)
32923306
TPM2_Packet_ParseU32(&packet, &paramSz);
32933307

32943308
TPM2_Packet_ParseU32(&packet, &out->digests.count);
3295-
for (i=0; (int)out->digests.count; i++) {
3309+
if (out->digests.count > HASH_COUNT) {
3310+
out->digests.count = HASH_COUNT;
3311+
}
3312+
for (i=0; i < (int)out->digests.count; i++) {
32963313
int digestSz;
32973314
TPM2_Packet_ParseU16(&packet, &out->digests.digests[i].hashAlg);
32983315
digestSz = TPM2_GetHashDigestSize(
@@ -4019,17 +4036,23 @@ static TPM_RC TPM2_PolicySessionOnly(TPM_CC cc, TPMI_SH_POLICY policy)
40194036

40204037
TPM_RC TPM2_PolicyPhysicalPresence(PolicyPhysicalPresence_In* in)
40214038
{
4039+
if (in == NULL)
4040+
return BAD_FUNC_ARG;
40224041
return TPM2_PolicySessionOnly(TPM_CC_PolicyPhysicalPresence,
40234042
in->policySession);
40244043
}
40254044

40264045
TPM_RC TPM2_PolicyAuthValue(PolicyAuthValue_In* in)
40274046
{
4047+
if (in == NULL)
4048+
return BAD_FUNC_ARG;
40284049
return TPM2_PolicySessionOnly(TPM_CC_PolicyAuthValue, in->policySession);
40294050
}
40304051

40314052
TPM_RC TPM2_PolicyPassword(PolicyPassword_In* in)
40324053
{
4054+
if (in == NULL)
4055+
return BAD_FUNC_ARG;
40334056
return TPM2_PolicySessionOnly(TPM_CC_PolicyPassword, in->policySession);
40344057
}
40354058

‎src/tpm2_asn.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,14 @@ int TPM2_ASN_DecodeRsaPubKey(uint8_t* input, int inputSz,
326326
if (rc == 0) {
327327
idx += mod_len;
328328
rc = TPM2_ASN_DecodeTag(input, inputSz, &idx, &exp_len, TPM2_ASN_INTEGER);
329+
}
330+
if (rc == 0) {
331+
/* Validate exp_len and idx before accessing input buffer */
332+
if (exp_len <= 0 || idx >= inputSz) {
333+
rc = -1;
334+
}
335+
}
336+
if (rc == 0) {
329337
if (input[idx] == 0x00) {
330338
idx++;
331339
exp_len--;

‎src/tpm2_cryptocb.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,8 +613,8 @@ int wolfTPM2_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
613613
if (hmacCtx && hmacCtx->hash.handle.hndl == 0) {
614614
#ifdef DEBUG_WOLFTPM
615615
printf("Error: HMAC context invalid!\n");
616-
return BAD_FUNC_ARG;
617616
#endif
617+
return BAD_FUNC_ARG;
618618
}
619619

620620
if (info->hmac.in != NULL) { /* Update */

‎src/tpm2_linux.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,12 @@ int TPM2_LINUX_SendCommand(TPM2_CTX* ctx, TPM2_Packet* packet)
144144
rspSz = (int)ret;
145145
rc = TPM_RC_SUCCESS;
146146
}
147-
else if (rspSz == 0) {
147+
else if (ret == 0) {
148148
#ifdef DEBUG_WOLFTPM
149149
printf("Received EOF(0) from %s: errno %d = %s\n",
150150
TPM2_LINUX_DEV, errno, strerror(errno));
151151
#endif
152+
rc = TPM_RC_FAILURE;
152153
}
153154
else {
154155
#ifdef DEBUG_WOLFTPM
@@ -190,6 +191,8 @@ int TPM2_LINUX_SendCommand(TPM2_CTX* ctx, TPM2_Packet* packet)
190191
printf("Response size: %d\n", (int)rspSz);
191192
TPM2_PrintBin(packet->buf, rspSz);
192193
}
194+
#else
195+
(void)rspSz;
193196
#endif
194197
return rc;
195198
}

‎src/tpm2_packet.c‎

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,10 @@ void TPM2_Packet_ParseBytes(TPM2_Packet* packet, byte* buf, int size)
191191
int sizeToCopy = size;
192192
if (packet->pos + sizeToCopy > packet->size)
193193
sizeToCopy = packet->size - packet->pos;
194-
XMEMCPY(buf, &packet->buf[packet->pos], sizeToCopy);
194+
/* Guard against negative sizeToCopy (when pos > size) */
195+
if (sizeToCopy > 0) {
196+
XMEMCPY(buf, &packet->buf[packet->pos], sizeToCopy);
197+
}
195198
}
196199
packet->pos += size;
197200
}
@@ -399,14 +402,32 @@ TPM_ST TPM2_Packet_AppendAuth(TPM2_Packet* packet, TPM2_CTX* ctx, CmdInfo_t* inf
399402

400403
void TPM2_Packet_ParseAuth(TPM2_Packet* packet, TPMS_AUTH_RESPONSE* authRsp)
401404
{
405+
UINT16 wireSize;
406+
402407
if (authRsp == NULL)
403408
return;
404409

405-
TPM2_Packet_ParseU16(packet, &authRsp->nonce.size);
410+
TPM2_Packet_ParseU16(packet, &wireSize);
411+
authRsp->nonce.size = wireSize;
412+
if (authRsp->nonce.size > sizeof(authRsp->nonce.buffer)) {
413+
authRsp->nonce.size = sizeof(authRsp->nonce.buffer);
414+
}
406415
TPM2_Packet_ParseBytes(packet, authRsp->nonce.buffer, authRsp->nonce.size);
416+
/* Skip any remaining bytes to keep packet position synchronized */
417+
if (wireSize > authRsp->nonce.size) {
418+
TPM2_Packet_ParseBytes(packet, NULL, wireSize - authRsp->nonce.size);
419+
}
407420
TPM2_Packet_ParseU8(packet, &authRsp->sessionAttributes);
408-
TPM2_Packet_ParseU16(packet, &authRsp->hmac.size);
421+
TPM2_Packet_ParseU16(packet, &wireSize);
422+
authRsp->hmac.size = wireSize;
423+
if (authRsp->hmac.size > sizeof(authRsp->hmac.buffer)) {
424+
authRsp->hmac.size = sizeof(authRsp->hmac.buffer);
425+
}
409426
TPM2_Packet_ParseBytes(packet, authRsp->hmac.buffer, authRsp->hmac.size);
427+
/* Skip any remaining bytes to keep packet position synchronized */
428+
if (wireSize > authRsp->hmac.size) {
429+
TPM2_Packet_ParseBytes(packet, NULL, wireSize - authRsp->hmac.size);
430+
}
410431
}
411432

412433
void TPM2_Packet_AppendPCR(TPM2_Packet* packet, TPML_PCR_SELECTION* pcr)
@@ -552,17 +573,35 @@ void TPM2_Packet_AppendEccPoint(TPM2_Packet* packet, TPMS_ECC_POINT* point)
552573
}
553574
void TPM2_Packet_ParseEccPoint(TPM2_Packet* packet, TPMS_ECC_POINT* point)
554575
{
576+
UINT16 wireSize;
577+
555578
if (point == NULL) {
556579
#ifdef DEBUG_WOLFTPM
557580
printf("Error null argument passed to TPM2_Packet_ParseEccPoint()\n");
558581
#endif
559582
return; /* help out static analysis */
560583
}
561584

562-
TPM2_Packet_ParseU16(packet, &point->x.size);
585+
TPM2_Packet_ParseU16(packet, &wireSize);
586+
point->x.size = wireSize;
587+
if (point->x.size > sizeof(point->x.buffer)) {
588+
point->x.size = sizeof(point->x.buffer);
589+
}
563590
TPM2_Packet_ParseBytes(packet, point->x.buffer, point->x.size);
564-
TPM2_Packet_ParseU16(packet, &point->y.size);
591+
/* Skip any remaining bytes to keep packet position synchronized */
592+
if (wireSize > point->x.size) {
593+
TPM2_Packet_ParseBytes(packet, NULL, wireSize - point->x.size);
594+
}
595+
TPM2_Packet_ParseU16(packet, &wireSize);
596+
point->y.size = wireSize;
597+
if (point->y.size > sizeof(point->y.buffer)) {
598+
point->y.size = sizeof(point->y.buffer);
599+
}
565600
TPM2_Packet_ParseBytes(packet, point->y.buffer, point->y.size);
601+
/* Skip any remaining bytes to keep packet position synchronized */
602+
if (wireSize > point->y.size) {
603+
TPM2_Packet_ParseBytes(packet, NULL, wireSize - point->y.size);
604+
}
566605
}
567606

568607
void TPM2_Packet_AppendPoint(TPM2_Packet* packet, TPM2B_ECC_POINT* point)
@@ -818,6 +857,7 @@ void TPM2_Packet_AppendSignature(TPM2_Packet* packet, TPMT_SIGNATURE* sig)
818857
void TPM2_Packet_ParseSignature(TPM2_Packet* packet, TPMT_SIGNATURE* sig)
819858
{
820859
int digestSz;
860+
UINT16 wireSize;
821861

822862
TPM2_Packet_ParseU16(packet, &sig->sigAlg);
823863

@@ -826,21 +866,54 @@ void TPM2_Packet_ParseSignature(TPM2_Packet* packet, TPMT_SIGNATURE* sig)
826866
case TPM_ALG_ECDAA:
827867
TPM2_Packet_ParseU16(packet, &sig->signature.ecdsa.hash);
828868

829-
TPM2_Packet_ParseU16(packet, &sig->signature.ecdsa.signatureR.size);
869+
TPM2_Packet_ParseU16(packet, &wireSize);
870+
sig->signature.ecdsa.signatureR.size = wireSize;
871+
if (sig->signature.ecdsa.signatureR.size >
872+
sizeof(sig->signature.ecdsa.signatureR.buffer)) {
873+
sig->signature.ecdsa.signatureR.size =
874+
sizeof(sig->signature.ecdsa.signatureR.buffer);
875+
}
830876
TPM2_Packet_ParseBytes(packet, sig->signature.ecdsa.signatureR.buffer,
831877
sig->signature.ecdsa.signatureR.size);
878+
/* Skip any remaining bytes to keep packet position synchronized */
879+
if (wireSize > sig->signature.ecdsa.signatureR.size) {
880+
TPM2_Packet_ParseBytes(packet, NULL,
881+
wireSize - sig->signature.ecdsa.signatureR.size);
882+
}
832883

833-
TPM2_Packet_ParseU16(packet, &sig->signature.ecdsa.signatureS.size);
884+
TPM2_Packet_ParseU16(packet, &wireSize);
885+
sig->signature.ecdsa.signatureS.size = wireSize;
886+
if (sig->signature.ecdsa.signatureS.size >
887+
sizeof(sig->signature.ecdsa.signatureS.buffer)) {
888+
sig->signature.ecdsa.signatureS.size =
889+
sizeof(sig->signature.ecdsa.signatureS.buffer);
890+
}
834891
TPM2_Packet_ParseBytes(packet, sig->signature.ecdsa.signatureS.buffer,
835892
sig->signature.ecdsa.signatureS.size);
893+
/* Skip any remaining bytes to keep packet position synchronized */
894+
if (wireSize > sig->signature.ecdsa.signatureS.size) {
895+
TPM2_Packet_ParseBytes(packet, NULL,
896+
wireSize - sig->signature.ecdsa.signatureS.size);
897+
}
836898
break;
837899
case TPM_ALG_RSASSA:
838900
case TPM_ALG_RSAPSS:
839901
TPM2_Packet_ParseU16(packet, &sig->signature.rsassa.hash);
840902

841-
TPM2_Packet_ParseU16(packet, &sig->signature.rsassa.sig.size);
903+
TPM2_Packet_ParseU16(packet, &wireSize);
904+
sig->signature.rsassa.sig.size = wireSize;
905+
if (sig->signature.rsassa.sig.size >
906+
sizeof(sig->signature.rsassa.sig.buffer)) {
907+
sig->signature.rsassa.sig.size =
908+
sizeof(sig->signature.rsassa.sig.buffer);
909+
}
842910
TPM2_Packet_ParseBytes(packet, sig->signature.rsassa.sig.buffer,
843911
sig->signature.rsassa.sig.size);
912+
/* Skip any remaining bytes to keep packet position synchronized */
913+
if (wireSize > sig->signature.rsassa.sig.size) {
914+
TPM2_Packet_ParseBytes(packet, NULL,
915+
wireSize - sig->signature.rsassa.sig.size);
916+
}
844917
break;
845918
case TPM_ALG_HMAC:
846919
TPM2_Packet_ParseU16(packet, &sig->signature.hmac.hashAlg);

0 commit comments

Comments
 (0)