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

DoS vulnerability via obsolete "css-what" version #1924

Closed
khitrenovich opened this issue Jun 8, 2021 · 11 comments
Closed

DoS vulnerability via obsolete "css-what" version #1924

khitrenovich opened this issue Jun 8, 2021 · 11 comments

Comments

@khitrenovich
Copy link

khitrenovich commented Jun 8, 2021 •
edited
Loading

There is fresh a security advisory on css-what in cheerio v0.22 - https://www.npmjs.com/advisories/1754.

Audit report -

yarn audit v1.22.10
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ css-what                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ cheerio                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ cheerio > css-select > css-what                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1754                        │
└───────────────┴──────────────────────────────────────────────────────────────┘

Note that css-what version that is currently in use is v2.1 and forced bump to v5 breaks cheerio - so that's not an option.
@fb55
Copy link
Member

fb55 commented Jun 8, 2021 •
edited
Loading

This issue only affects css-what@4.0.0 and higher. I am trying to get the CVE updated to reflect this.

For now, I have published cheerio@1.0.0-rc.10, which includes the version update.

@khitrenovich
Copy link
Author

@fb55 I hope you'll manage to update the CVE. We use fragments in our project, so v1.0 is not a drop-in replacement.

@fb55
Copy link
Member

fb55 commented Jun 8, 2021

Sent an email, let's see how long this will take.

Out of curiosity: What do you mean with fragments? The module itself hasn't changed too much, so curious to hear how we can improve the adoption story.

@khitrenovich
Copy link
Author

Sent an email, let's see how long this will take.

Last time I had to do something like that, it was a matter of hours for the advisory team to take care.

Out of curiosity: What do you mean with fragments? The module itself hasn't changed too much, so curious to hear how we can improve the adoption story.

We are manipulating HTML templates, which can be each full document (with <html>/<head>/<body>) or fragments (single <div> elements, for example). We "patch" certain elements with either generated content or content from another templates. In v0.x, $.html() returned either fragment or the full document, depending on what was initially loaded. In v1.0 we always get back full document wrapped with <html> etc.

That's what I see in the unit test failure, for example -

       -   "template": "<div id=\"content\"><div>email content</div></div>",
       +   "template": "<html><head></head><body><div id=\"content\"><div>email content</div></div></body></html>",

@meendoo meendoo mentioned this issue Jun 9, 2021
Closed
@fb55
Copy link
Member

fb55 commented Jun 9, 2021

In v1.0 we always get back full document wrapped with etc.

All you'll have to do to disable this behavior is set the isDocument boolean of load to false:

const $ = cheerio.load(htmlString, null, false)

@davidowens
Copy link

This issue only affects css-what@4.0.0

@fb55 can you point me to where you found that?

@fb55
Copy link
Member

fb55 commented Jun 9, 2021

@fb55 can you point me to where you found that?

I am the maintainer of css-what. The bug was introduced in fb55/css-what@63cb253

This was referenced Jun 9, 2021
Merged
Closed
@davidowens
Copy link

awesome thank you!

@khitrenovich
Copy link
Author

All you'll have to do to disable this behavior is set the isDocument boolean of load to false:

const $ = cheerio.load(htmlString, null, false)

Great, this is very helpful! I think it is worth explicitly documenting that flag in "migrating from v0.x" section.

@ljharb ljharb mentioned this issue Jun 17, 2021
Closed
@edavidsonsawyer edavidsonsawyer mentioned this issue Jun 17, 2021
Merged
@prateekm21 prateekm21 mentioned this issue Jun 28, 2021
Open
@radium-v radium-v mentioned this issue Jun 28, 2021
Closed
9 tasks
@s0ph1e s0ph1e mentioned this issue Jun 30, 2021
Closed
@CRD716 CRD716 mentioned this issue Jul 13, 2021
Open
@curvedriver
Copy link

This issue only affects css-what@4.0.0 and higher. I am trying to get the CVE updated to reflect this.

For now, I have published cheerio@1.0.0-rc.10, which includes the version update.

FYI, I requested an update of the npm advisory so that the version list is equal to the CVE and npm audit no longer reports the security vulnerability.

https://npmjs.com/advisories/1754

@fb55
Copy link
Member

fb55 commented Jul 16, 2021

Thanks @curvedriver! Closing this as everything seems to be taken care of.

@fb55 fb55 closed this as completed Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fb55 @davidowens @khitrenovich @curvedriver