Skip to content

fix set_cipherlist edge case#6519

Closed
miyazakh wants to merge 2 commits intowolfSSL:masterfrom
miyazakh:set_cipherlist
Closed

fix set_cipherlist edge case#6519
miyazakh wants to merge 2 commits intowolfSSL:masterfrom
miyazakh:set_cipherlist

Conversation

@miyazakh
Copy link
Copy Markdown
Contributor

@miyazakh miyazakh commented Jun 17, 2023

Description

CC-RX malloc returns NULL when its size is zero. This causes wolfSSL_CTX_set_cipher_list failure in edge case.

./configure --enable-opensslextra --enable-smallstack

Testing

Run unit test case
Confirm the fix by CC-RX

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@miyazakh miyazakh self-assigned this Jun 17, 2023
@miyazakh
Copy link
Copy Markdown
Contributor Author

retest this please

@miyazakh miyazakh closed this Jun 28, 2023
@miyazakh miyazakh reopened this Jun 28, 2023
@miyazakh miyazakh closed this Jun 30, 2023
@miyazakh miyazakh reopened this Jun 30, 2023
@miyazakh miyazakh assigned cconlon and unassigned miyazakh Jul 1, 2023
Comment thread src/ssl.c
suitesCpy = (byte*)XMALLOC(suites->suiteSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (suitesCpy == NULL)
return WOLFSSL_FAILURE;
if (suitesCpy == NULL) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little odd to me, that if suites->suiteSz is 0 that we would allocate suitesCpy to a size of 1.

Is this the correct way to handle this, or should we just be skipping the XMALLOC all together if suites->suiteSz is 0? Then adjusting logic inside this function to accomodate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. I think that just skipping the XMALLOC when suites->suiteSz is zero is better than the logic here. will check that with CC-RX

@cconlon cconlon assigned miyazakh and unassigned cconlon Sep 14, 2023
@miyazakh
Copy link
Copy Markdown
Contributor Author

@cconlon

This is fixed by PR#6904

@miyazakh miyazakh closed this Dec 13, 2023
@miyazakh miyazakh deleted the set_cipherlist branch May 28, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants