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

src: guard against overflow in ParseArrayIndex() (v4.x) #8409

Closed
wants to merge 3 commits into from
Closed

src: guard against overflow in ParseArrayIndex() (v4.x) #8409

wants to merge 3 commits into from

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Sep 5, 2016 •
edited
Loading

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Sep 5, 2016
@mscdex mscdex added v4.x buffer Issues and PRs related to the buffer subsystem. labels Sep 5, 2016
@jasnell
Copy link
Member

jasnell commented Sep 7, 2016

Some red in the CI

@bnoordhuis
Copy link
Member Author

It's that pseudo-tty/no_dropped_stdio test. I don't think that's caused by this PR.

@MylesBorins
Copy link
Contributor

MylesBorins commented Sep 7, 2016 •
edited
Loading

Here is a run of CI on the head of v4.x-staging for comparison https://ci.nodejs.org/job/node-test-commit/4950/

@MylesBorins
Copy link
Contributor

@bnoordhuis would yu be able to rebase?

@MylesBorins MylesBorins force-pushed the v4.x-staging branch 2 times, most recently from 6a126fb to 004d665 Compare October 10, 2016 22:18
bnoordhuis and others added 3 commits October 19, 2016 13:35
@bnoordhuis
src: move ParseArrayIndex() to src/node_buffer.cc
31ea3a5 
It's not used anywhere else so move it out of src/node_internals.h.

PR-URL: nodejs#7497
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis
src: guard against overflow in ParseArrayIndex()
6e62d9f 
ParseArrayIndex() would wrap around large (>=2^32) index values on
platforms where sizeof(int64_t) > sizeof(size_t).  Ensure that the
return value fits in a size_t.

PR-URL: nodejs#7497
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig @bnoordhuis
src: suppress coverity message
785b91e 
Coverity marked a change in 630096b as a constant expression.
However, on platforms where sizeof(int64_t) > sizeof(size_t),
this should not be the case. This commit flags the comparison
as OK to coverity.

PR-URL: nodejs#7587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@bnoordhuis
Copy link
Member Author

@thealphanerd Rebased, PTAL. CI: https://ci.nodejs.org/job/node-test-pull-request/4573/

@MylesBorins MylesBorins added this to the v4.6.2 milestone Oct 24, 2016
@MylesBorins
Copy link
Contributor

CI is good... only failures are infra. landing

@MylesBorins
Copy link
Contributor

landed in 29315da...f1a7a1a

All CI failures were infra related

This was referenced Mar 9, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
4.6.2
Development

Successfully merging this pull request may close these issues.
6 participants
@bnoordhuis @jasnell @MylesBorins @mscdex @nodejs-github-bot @cjihrig