New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[apps/speed] Added checking for buflen overflow due to MAX_MISALIGNMENT. #17646
Changes from 3 commits
aab22e8
9ca2706
145b353
2033be3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -455,7 +455,7 @@ static const OPT_PAIR sm2_choices[SM2_NUM] = { | |
static double sm2_results[SM2_NUM][2]; /* 2 ops: sign then verify */ | ||
#endif /* OPENSSL_NO_SM2 */ | ||
|
||
#define COND(unused_cond) (run && count < 0x7fffffff) | ||
#define COND(unused_cond) (run && count < INT_MAX) | ||
#define COUNT(d) (count) | ||
|
||
typedef struct loopargs_st { | ||
|
@@ -1778,7 +1778,13 @@ int speed_main(int argc, char **argv) | |
buflen = lengths[size_num - 1]; | ||
if (buflen < 36) /* size of random vector in RSA benchmark */ | ||
buflen = 36; | ||
buflen += MAX_MISALIGNMENT + 1; | ||
if (INT_MAX - (MAX_MISALIGNMENT + 1) < buflen) | ||
{ | ||
BIO_printf(bio_err, "Error: buffer size too large\n"); | ||
goto end; | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because each branch of the if is but a single statement, our coding style would suggest omitting the {} on both. I think better might be changing the warning to an error and failing. In which case, the {} should be retained. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paulidale Made it an error, also updated help with the maximum possible buffer size for clarity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could actually be not an "else" at all and just unconditional since the main branch ends in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed this as well. |
||
buflen += MAX_MISALIGNMENT + 1; | ||
} | ||
loopargs[i].buf_malloc = app_malloc(buflen, "input buffer"); | ||
loopargs[i].buf2_malloc = app_malloc(buflen, "input buffer"); | ||
memset(loopargs[i].buf_malloc, 0, buflen); | ||
|
@@ -3612,7 +3618,7 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher, int lengths_single, | |
for (j = 0; j < num; j++) { | ||
print_message(alg_name, 0, mblengths[j], seconds->sym); | ||
Time_F(START); | ||
for (count = 0; run && count < 0x7fffffff; count++) { | ||
for (count = 0; run && count < INT_MAX; count++) { | ||
unsigned char aad[EVP_AEAD_TLS1_AAD_LEN]; | ||
EVP_CTRL_TLS1_1_MULTIBLOCK_PARAM mb_param; | ||
size_t len = mblengths[j]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Placement of
{
should be on the end of the preceding lineThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed this.