Skip to content

Commit

Permalink
Fix constant-time comparison of negative values
Browse files Browse the repository at this point in the history
Thanks Coverity CID 414687
  • Loading branch information
xhanulik committed Feb 7, 2024
1 parent 5747804 commit c153e2f
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
6 changes: 6 additions & 0 deletions src/common/constant-time.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,10 @@ constant_time_eq_s(size_t a, size_t b)
return constant_time_is_zero_s(a ^ b);
}

static constant_inline unsigned int
constant_time_eq_i(int a, int b)
{
return constant_time_eq((unsigned int)a, (unsigned int)b);
}

#endif /* CONSTANT_TIME_H */
4 changes: 2 additions & 2 deletions src/minidriver/minidriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -4660,7 +4660,7 @@ DWORD WINAPI CardRSADecrypt(__in PCARD_DATA pCardData,
logprintf(pCardData, 2, "sc_pkcs15_decipher: stripping PKCS1 padding\n");
r = sc_pkcs1_strip_02_padding_constant_time(vs->ctx, prkey_info->modulus_length / 8, pbuf2, pInfo->cbData, pbuf2, &temp);
pInfo->cbData = (DWORD) temp;
wrong_padding = constant_time_eq_s(r, SC_ERROR_WRONG_PADDING);
wrong_padding = constant_time_eq_i(r, SC_ERROR_WRONG_PADDING);
/* continue without returning error to not leak that padding is wrong
to prevent time side-channel leak for Marvin attack*/
}
Expand Down Expand Up @@ -4710,7 +4710,7 @@ DWORD WINAPI CardRSADecrypt(__in PCARD_DATA pCardData,
goto err;
}

good = constant_time_eq_s(r, 0);
good = constant_time_eq_i(r, 0);
/* if no error or padding error, do not return here to prevent Marvin attack */
if (!(good | wrong_padding) && r < 0) {
logprintf(pCardData, 2, "sc_pkcs15_decipher error(%i): %s\n", r, sc_strerror(r));
Expand Down
4 changes: 2 additions & 2 deletions src/pkcs11/framework-pkcs15.c
Original file line number Diff line number Diff line change
Expand Up @@ -4627,11 +4627,11 @@ pkcs15_prkey_decrypt(struct sc_pkcs11_session *session, void *obj,

/* only padding error must be handled in constant-time way,
* other error can be returned straight away */
if ((~constant_time_eq_s(rv, SC_ERROR_WRONG_PADDING) & constant_time_lt_s(sizeof(decrypted), rv)))
if ((~constant_time_eq_i(rv, SC_ERROR_WRONG_PADDING) & constant_time_lt_s(sizeof(decrypted), (size_t)rv)))
return sc_to_cryptoki_error(rv, "C_Decrypt");

/* check rv for padding error */
good = ~constant_time_eq_s(rv, SC_ERROR_WRONG_PADDING);
good = ~constant_time_eq_i(rv, SC_ERROR_WRONG_PADDING);
rv_pkcs11 = sc_to_cryptoki_error(SC_ERROR_WRONG_PADDING, "C_Decrypt");
rv_pkcs11 = constant_time_select_s(good, CKR_OK, rv_pkcs11);

Expand Down

0 comments on commit c153e2f

Please sign in to comment.