Skip to content
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

Jump to bottom

src: replace unreachable code with static_assert #46209

Merged
merged 1 commit into from Jan 17, 2023
Merged

src: replace unreachable code with static_assert #46209

merged 1 commit into from Jan 17, 2023

Conversation

tniessen
Copy link
Member

This function base64-decodes a given JavaScript string to obtain the secret key, whose length must not exceed INT_MAX. However, because JavaScript strings are limited to v8::String::kMaxLength chars and because base64 decoding never yields more bytes than input chars, the size of the decoded key must be strictly less than v8::String::kMaxLength bytes. Therefore, it is sufficient to statically assert that String::kMaxLength <= INT_MAX (which is always true because String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code "suspicious" because it indicates that buffers larger than INT_MAX might actually be allocated.

@tniessen
src: replace unreachable code with static_assert
07aa9af 
This function base64-decodes a given JavaScript string to obtain the
secret key, whose length must not exceed INT_MAX. However, because
JavaScript strings are limited to v8::String::kMaxLength chars and
because base64 decoding never yields more bytes than input chars, the
size of the decoded key must be strictly less than
v8::String::kMaxLength bytes. Therefore, it is sufficient to statically
assert that String::kMaxLength <= INT_MAX (which is always true because
String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code
"suspicious" because it indicates that buffers larger than INT_MAX might
actually be allocated.
@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Jan 14, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 14, 2023
@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 17, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 17, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49004/

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 17, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 17, 2023
@nodejs-github-bot nodejs-github-bot merged commit 15d673d into nodejs:main Jan 17, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 15d673d

RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Jan 17, 2023
@tniessen @RafaelGSS
src: replace unreachable code with static_assert
dbcafab 
This function base64-decodes a given JavaScript string to obtain the
secret key, whose length must not exceed INT_MAX. However, because
JavaScript strings are limited to v8::String::kMaxLength chars and
because base64 decoding never yields more bytes than input chars, the
size of the decoded key must be strictly less than
v8::String::kMaxLength bytes. Therefore, it is sufficient to statically
assert that String::kMaxLength <= INT_MAX (which is always true because
String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code
"suspicious" because it indicates that buffers larger than INT_MAX might
actually be allocated.

PR-URL: nodejs#46209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@github-actions github-actions bot mentioned this pull request Jan 18, 2023
Open
18 tasks
tniessen added a commit to tniessen/node that referenced this pull request Jan 18, 2023
@tniessen
src: replace unreachable code with static_assert
632c5b4 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: nodejs#46209
@tniessen tniessen mentioned this pull request Jan 18, 2023
Merged
tniessen added a commit to tniessen/node that referenced this pull request Jan 18, 2023
@tniessen
src: replace unreachable code with static_assert
2eb372d 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: nodejs#46209
This was referenced Jan 19, 2023
Open
Open
RafaelGSS pushed a commit that referenced this pull request Jan 20, 2023
@tniessen @RafaelGSS
src: replace unreachable code with static_assert
36ae3cc 
This function base64-decodes a given JavaScript string to obtain the
secret key, whose length must not exceed INT_MAX. However, because
JavaScript strings are limited to v8::String::kMaxLength chars and
because base64 decoding never yields more bytes than input chars, the
size of the decoded key must be strictly less than
v8::String::kMaxLength bytes. Therefore, it is sufficient to statically
assert that String::kMaxLength <= INT_MAX (which is always true because
String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code
"suspicious" because it indicates that buffers larger than INT_MAX might
actually be allocated.

PR-URL: #46209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 20, 2023
Merged
nodejs-github-bot pushed a commit that referenced this pull request Jan 21, 2023
@tniessen
src: replace unreachable code with static_assert
5ccf4ef 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: #46209
PR-URL: #46250
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
@tniessen @juanarbol
src: replace unreachable code with static_assert
f800be5 
This function base64-decodes a given JavaScript string to obtain the
secret key, whose length must not exceed INT_MAX. However, because
JavaScript strings are limited to v8::String::kMaxLength chars and
because base64 decoding never yields more bytes than input chars, the
size of the decoded key must be strictly less than
v8::String::kMaxLength bytes. Therefore, it is sufficient to statically
assert that String::kMaxLength <= INT_MAX (which is always true because
String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code
"suspicious" because it indicates that buffers larger than INT_MAX might
actually be allocated.

PR-URL: #46209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@juanarbol juanarbol mentioned this pull request Jan 28, 2023
Merged
juanarbol pushed a commit that referenced this pull request Jan 31, 2023
@tniessen @juanarbol
src: replace unreachable code with static_assert
94605b1 
This function base64-decodes a given JavaScript string to obtain the
secret key, whose length must not exceed INT_MAX. However, because
JavaScript strings are limited to v8::String::kMaxLength chars and
because base64 decoding never yields more bytes than input chars, the
size of the decoded key must be strictly less than
v8::String::kMaxLength bytes. Therefore, it is sufficient to statically
assert that String::kMaxLength <= INT_MAX (which is always true because
String::kMaxLength itself is an int).

Aside from being unreachable, Coverity considers the current code
"suspicious" because it indicates that buffers larger than INT_MAX might
actually be allocated.

PR-URL: #46209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
ruyadorno pushed a commit that referenced this pull request Feb 1, 2023
@tniessen @ruyadorno
src: replace unreachable code with static_assert
8307a4b 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: #46209
PR-URL: #46250
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
@tniessen @juanarbol
src: replace unreachable code with static_assert
9cd0a1d 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: #46209
PR-URL: #46250
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
@tniessen @juanarbol
src: replace unreachable code with static_assert
cde3755 
This function divides an unsigned 32-bit integer by 8, effectively
right-shifting it by three bits, so the result must be less than
INT_MAX.

Refs: #46209
PR-URL: #46250
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panva panva panva approved these changes

@Trott Trott Trott approved these changes

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

@cjihrig cjihrig cjihrig approved these changes

@JungMinu JungMinu JungMinu approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@tniessen @nodejs-github-bot @panva @Trott @lpinca @anonrig @cjihrig @JungMinu @RaisinTen