-
Notifications
You must be signed in to change notification settings - Fork 30k
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
deps: update chromium zlib 1.2.12 #42571
deps: update chromium zlib 1.2.12 #42571
Conversation
@@ -17,7 +17,7 @@ | |||
#if !defined(CHROMIUM_ZLIB_NO_CHROMECONF) | |||
/* This include does prefixing as below, but with an updated set of names. Also | |||
* sets up export macros in component builds. */ | |||
//#include "chromeconf.h" | |||
#include "chromeconf.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change still needs to be backed out IIRC. Either that, or you might try adding defining CHROMIUM_ZLIB_NO_CHROMECONF
in the gyp file to see if that works. I commented this out in my original PR that added the Chromium zlib implementation because it was breaking zlib tests.
@@ -163,4 +163,4 @@ | |||
], | |||
}], | |||
], | |||
} | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with stripping out trailing spaces in this PR rather than leaving them or doing it in a separate PR.
By the looks of it, the gyp will probably need to be updated to match the changes made to the build.gn file, especially for any optimization-related changes. |
Feel free to look at what I did in https://github.com/targos/node/tree/update-zlib-2 to fix the gyp config. |
am I correct to assume nodejs is vulnerable to CVE-2018-25032 until this is integrated/released? |
What would be the plan to close CVE-2018-25032 for the latest node LTS versions so that security scanning tools stop reporting the zlib implementation in node as vulnerable? I understand from the linked comments in #31201 that Chromium zlib which is being used is not vulnerable, but how to update nodejs such that this manual review of the situation and analysis is not required? |
Note there is now a 1.2.13 with CVE-2022-37434 fix. |
Superseded by #45387. |
Updates chromium zlib to 1.2.12
kept zlib.gyp build file.
https://chromium.googlesource.com/chromium/src/third_party/zlib/+/faff052b6b6edcd6dd548513fe44ac0941427bf0