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

[0.10] deps: update libuv to 0.10.37 #7293

Closed
wants to merge 1 commit into from

Conversation

saghul
Copy link
Member

@saghul saghul commented Jun 13, 2016

Fixes: #7199
Refs: #2723

/cc @nodejs/collaborators

@nodejs-github-bot nodejs-github-bot added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Jun 13, 2016
@saghul
Copy link
Member Author

saghul commented Jun 13, 2016

@rvagg
Copy link
Member

rvagg commented Jun 13, 2016

test-stdout-close-catch failure on Windows is the only suspicious one I see in there. https://ci.nodejs.org/job/node-test-commit/3737/ for good measure.

@saghul is there anything else in here that we should be concerned about? I'm thinking that if we ship this in our security update this week what is the potential for unintended breakages beyond just the rwlocks fix?

@rvagg
Copy link
Member

rvagg commented Jun 13, 2016

Bad params for that CI run, trying again @ https://ci.nodejs.org/job/node-test-commit/3738/

Smoke testing @ https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/302/

@rvagg
Copy link
Member

rvagg commented Jun 14, 2016

two test-stdout-close-catch failures on Windows this time, that's not so great

@saghul
Copy link
Member Author

saghul commented Jun 14, 2016

I don't see how the changes in this release can affect stdout related tests :-( The rwlock related fix looks big but it's not a big deal, and not used for stdio stuff AFAIK.

IMHO this is a low impact update.

Do those tests consistently pass without this patch?

@rvagg
Copy link
Member

rvagg commented Jun 14, 2016

ran on vanilla v0.10-staging and got a failure of the same test https://ci.nodejs.org/job/node-test-binary-windows/2518/

there's so much flakiness on v0.10 and v0.12 that it's really hard to tell what's a cause for concern and what's not, I'll be glad when we retire all of that at the end of the year.

this update lgtm

@saghul
Copy link
Member Author

saghul commented Jun 14, 2016

Thanks for confirming. I'll wait until wednesday to merge in case someone wants to take a look.

@mhdawson
Copy link
Member

I'd still vote for not including in the security release. I think security releases should just be the previous release plus the security changes, at least for the LTS lines.

Is there some reason this fix is important enough to sneak it in ?

@saghul
Copy link
Member Author

saghul commented Jun 14, 2016

@mhdawson This includes a security fix that affects Windows XP. I know, I know.

@rmg
Copy link
Contributor

rmg commented Jun 14, 2016

If one tries hard, you could consider the missing ENFILE mapping a DOS vector since prior to this libuv they were treated as unknown errors. Have to squint and wave your arms, a lot, though.

@rvagg
Copy link
Member

rvagg commented Jun 15, 2016

We considered this a security issue in the past. See libuv/libuv#515 for the upstream discussion and #2723 where we fixed the bits in Node that used rwlocks and labelled this CVE-2014-9748 (Mitre hasn't processed the memo that it should be public, we're working on that again).

I'm fine with deferring this until the next release of v0.10 if nobody sees any urgency in here (I'm relying on libuv folk here for this assessment, @bnoordhuis, @saghul).

@saghul
Copy link
Member Author

saghul commented Jun 15, 2016

I'd say we include this in tomorrow's release. It's a low risk change which we neglected enough already. To be honest, if this wasn't going on tomorrow's release I would have neglected it even longer.

People running 0.10 are probably not used to many Node updates by now, so immediately queuing up 2 releases sounds pointless to me.

@rvagg
Copy link
Member

rvagg commented Jun 23, 2016

landed in node-private, will be merged here tomorrow

@rvagg rvagg closed this Jun 23, 2016
rvagg added a commit that referenced this pull request Jun 23, 2016
This is a security release. All Node.js users should consult the security
release summary at
https://nodejs.org/en/blog/vulnerability/june-2016-security-releases/ for
details on patched vulnerabilities.

Notable changes:

* libuv: (CVE-2014-9748) Fixes a bug in the read/write locks implementation for
  Windows XP and Windows 2003 that can lead to undefined and potentially unsafe
  behaviour. More information can be found at
  libuv/libuv#515 or at
  https://nodejs.org/en/blog/vulnerability/june-2016-security-releases/.
* V8: (CVE-2016-1669) Fixes a potential Buffer overflow vulnerability discovered
  in V8, more details can be found in the CVE CVE-2016-1669 at
  https://www.cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-1669 or at
  https://nodejs.org/en/blog/vulnerability/june-2016-security-releases/.

Commits:

* [3374f57] - deps: update libuv to 0.10.37 (Saúl Ibarra Corretgé) #7293
* [fcb9145] - deps: backport 3a9bfec from v8 upstream (Myles Borins) nodejs-private/node-private#43

PR-URL: nodejs-private/node-private#52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants