Skip to content

20240327-tls-int-overflows#7371

Merged
dgarske merged 1 commit intowolfSSL:masterfrom
douzzer:20240327-tls-int-overflows
Mar 29, 2024
Merged

20240327-tls-int-overflows#7371
dgarske merged 1 commit intowolfSSL:masterfrom
douzzer:20240327-tls-int-overflows

Conversation

@douzzer
Copy link
Copy Markdown
Contributor

@douzzer douzzer commented Mar 28, 2024

src/internal.c: mitigations for potential integer overflows in figuring allocation sizes.

tested with wolfssl-multi-test.sh ... super-quick-check

see ZD#17733

Comment thread src/internal.c Outdated
@douzzer douzzer requested a review from SparkiDev March 28, 2024 05:14
Comment thread src/internal.c Outdated
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 logic is incredibly difficult to read and seems to make some assumption about overflow and size_t, which can be 32-bit or 64-bit depending on the platform. Please re-write this and avoid using size_t.

Comment thread src/internal.c Outdated
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.

Why was this changed?

@douzzer douzzer force-pushed the 20240327-tls-int-overflows branch from 74d6808 to beb6748 Compare March 29, 2024 05:41
@douzzer douzzer requested a review from dgarske March 29, 2024 05:41
@douzzer douzzer assigned dgarske and unassigned douzzer and wolfSSL-Bot Mar 29, 2024
Comment thread src/internal.c Outdated
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.

c:\jenkins\workspace\prb-windows-test-v2\src\internal.c(10583): warning C4701: potentially uninitialized local variable 'newSz' used [C:\jenkins\workspace\PRB-windows-test-v2\wolfssl.vcxproj] Lib

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.

good catch! it's a false positive, but we don't want any positives, false or otherwise.

I'll tweak the macro.

@dgarske dgarske assigned douzzer and unassigned dgarske Mar 29, 2024
src/internal.c: mitigations for potential integer overflows in figuring allocation sizes.
@douzzer douzzer force-pushed the 20240327-tls-int-overflows branch from beb6748 to 038be95 Compare March 29, 2024 16:45
@douzzer douzzer assigned dgarske and unassigned douzzer Mar 29, 2024
@douzzer douzzer requested a review from dgarske March 29, 2024 18:35
@dgarske dgarske merged commit 5c486cb into wolfSSL:master Mar 29, 2024
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.

4 participants