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.

Sign up for GitHub

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

crypto: fix checkPrime crash with large buffers #56559

Closed
wants to merge 1 commit into from
Closed

crypto: fix checkPrime crash with large buffers #56559

wants to merge 1 commit into from

Conversation

santigimeno
Copy link
Member

Fixes: #56512

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added 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. labels Jan 11, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 11, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.18%. Comparing base (3946f16) to head (2774611).
Report is 25 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #56559      +/-   ##
==========================================
+ Coverage   89.16%   89.18%   +0.01%     
==========================================
  Files         662      662              
  Lines      191751   191799      +48     
  Branches    36900    36917      +17     
==========================================
+ Hits       170972   171053      +81     
+ Misses      13635    13581      -54     
- Partials     7144     7165      +21
Files with missing lines Coverage Δ
src/crypto/crypto_random.cc 75.22% <100.00%> (+1.19%) ⬆️

... and 34 files with indirect coverage changes

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 13, 2025 •
edited by jasnell
Loading

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

@santigimeno santigimeno added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 13, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 13, 2025
@nodejs-github-bot

This comment was marked as duplicate.

Sign in to view
mertcanaltin
mertcanaltin approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@mertcanaltin mertcanaltin left a comment

Choose a reason for hiding this comment

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

lgtm

@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 13, 2025
jasnell pushed a commit that referenced this pull request Jan 13, 2025
@santigimeno @jasnell
crypto: fix checkPrime crash with large buffers
7a9d783 
Fixes: #56512
PR-URL: #56559
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jan 13, 2025

Landed in 7a9d783

@jasnell jasnell closed this Jan 13, 2025
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Jan 13, 2025
@santigimeno @Ceres6
crypto: fix checkPrime crash with large buffers
6e61361 
Fixes: nodejs#56512
PR-URL: nodejs#56559
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mertcanaltin mertcanaltin mertcanaltin approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell 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.

crypto.checkPrime results in an abort
6 participants
@santigimeno @nodejs-github-bot @jasnell @cjihrig @joyeecheung @mertcanaltin