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

missing release notes for CVE-2020-10531 #3245

Closed
cedricbu opened this issue Jun 11, 2020 · 12 comments
Closed

missing release notes for CVE-2020-10531 #3245

cedricbu opened this issue Jun 11, 2020 · 12 comments

Comments

@cedricbu
Copy link

Hello,

CVE-2020-10531 was silently fixed in 14.3.0 and v14.3.0 , as a side effect of ICU rebase.

fix in v12.17.0 : nodejs/node@2d76ae7#diff-bd007b7962bfa64fcd3a1e11d91f03daL1566

14.3.0 : nodejs/node@331f0b3#diff-bd007b7962bfa64fcd3a1e11d91f03daL1566

Could you please update the corresponding release notes to reflect this fact ? (currently, from a release notes point of view, only 10.21.0 has an acknowledged fix)

@cedricbu
Copy link
Author

Original upstream ICU fix for the CVE: unicode-org/icu@b7d08bc

@nschonni
Copy link
Member

I think these were called out in the separate Security Release blog post https://nodejs.org/en/blog/vulnerability/june-2020-security-releases/
Not sure how the previous ones were handled, but I guess you're suggesting that block at the top of the v10 blog post gets copied over to the others?

@cedricbu
Copy link
Author

yes, these were called out in the June 2020 security releases ... but that release note states : "Does not affect 12.x or 14.x, they do not include an affected version of ICU." ... which is true, ... but the reason that it is true is because it was silently fixed in their respective previous version.

I guess the release notes for v12.17.0 & v14.3.0 could be modified to mention that they are fixing CVE-2020-10531, and the June 2020 notes could be modified to add a pointer to these 2 previous releases as the fixed version.

The issue is that, currently, a person reading the release notes might wrongly interpret that node-12 and node-14 were never vulnerable to CVE-2020-10531

@nschonni
Copy link
Member

@nodejs/releasers maybe you can answer this one, since you folks usually cut the release note PRs

@jaas666
Copy link
Contributor

jaas666 commented Jun 14, 2022

Trying to tidy up this place and tying loose ends.
Is this still relevant or can it be closed?

@cedricbu
Copy link
Author

The release notes for v12.17.0 and v14.3.0 still do not mention that they contain the fix for CVE-2020-10531.

To re-summarize the issue :
https://nodejs.org/en/blog/vulnerability/june-2020-security-releases/ states that, quote, CVE-2020-10531 "Does not affect 12.x or 14.x, they do not include an affected version of ICU." ... That is not entirely true : node-12 < 12.17.0 and node-14 < 14.3.0 included a vulnerable version of ICU (i.e.: CVE-2020-10531 affected those versions of node).

Someone running for example node 14.2.0 may wrongfully believe to not be affected by CVE-2020-10531, based on the lack of public details (no mention of the fix in the 14.3.0 & 12.17.0 release notes, and the notes from June 2020 security release may mislead people into thinking that 12 and 14 were never affected).

I am not sure whether or not it should be considered relevant.... But I prefer when security notes are tidy and clean

@ovflowd
Copy link
Member

ovflowd commented Aug 15, 2022

That is not entirely true : node-12 < 12.17.0 and node-14 < 14.3.0 included a vulnerable version of ICU (i.e.: GHSA-8xp2-qvq2-xhpx affected those versions of node).

Could you show the logs of where it states that these versions were used? Otherwise, if our Release team mentions that those versions are not affected, I don't see any reason for invalidating their claims.

@cedricbu
Copy link
Author

I am not sure where to find that in the logs, but I can show you that in the code.

In v12.16.0 (which is < 12.17.0), we can find the vulnerable code : https://github.com/nodejs/node/blob/v12.16.0/deps/icu-small/source/common/unistr.cpp#L1565

As a reminder, according to NIST (https://nvd.nist.gov/vuln/detail/CVE-2020-10531) the ICU upstream fix is there : unicode-org/icu@b7d08bc#diff-7d047c27f750d3f742e9cc532e4d688fcf8618575975426cb305d727e105b0d3

if you compare the 2 links, you will see that the file source/common/unistr.cpp shipped in nodejs v12.16.0 is vulnerable (it does a simple addition instead of the uprv_add32_overflow() function), which contradicts the node.js release statement regarding that CVE (https://nodejs.org/en/blog/vulnerability/june-2020-security-releases/#icu-20958-prevent-segv_maperr-in-append-high-cve-2020-10531), which state that it "Does not affect 12.x or 14.x, they do not include an affected version of ICU."

@cedricbu
Copy link
Author

cedricbu commented Aug 16, 2022

@ovflowd
Copy link
Member

ovflowd commented Aug 18, 2022

Gotcha, it's been such a long time that I'm not sure it is worth chasing after the rabbit, but maybe @nodejs/release might be able to assess if the blog post should be updated.

@cedricbu
Copy link
Author

Yes, I agree. Back when I reported the issue, it might have mattered for people running a vulnerable version (e.g.: it might have weighted in favor of an upgrade of node.js).
... but now, in 2022, anyone still running such an old version is vulnerable to many CVEs, such that one more additional undocumented vulnerability does not really matter

(That being said, if I can express my humble opinion: security release notes should always be correct, as they they tend to be considered as a source of truth)

@ovflowd
Copy link
Member

ovflowd commented Aug 18, 2022

(That being said, if I can express my humble opinion: security release notes should always be correct, as they they tend to be considered as a source of truth)

Noted! Thank you for the valuable info and help! And I apologise for the misleading endeavours!

Closing this issue as there are no actions to be done for now.

@ovflowd ovflowd closed this as completed Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants