jonkemp / inline-css

Inline css into an html file.
MIT License
429 stars 85 forks source link

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

Open prateekm21 opened 3 years ago

prateekm21 commented 3 years ago

Hello Maintainers,

I want to get this on your radar: npm audit is failing for vulnerability in css-what package

I saw cheerio is working on fixing it here and looking forward forinline-css to incorporate this update and resolve vulnerability

High Denial of Service
Package css-what
Patched in >=5.0.1
Dependency of inline-css
Path inline-css > cheerio > css-select > css-what
More Info https://npmjs.com/advisories/1754
mike-dvorscak-enablon commented 3 years ago

I recently upgraded css-what for a project that I am working on. Part of the fix included forcing the resolution of cheerio to 1.0.0.rc-10. However, this version of cheerio seems to be incompatible with the current version of inline-css, now the following error always appears when requiring inline-css:

image

Which comes from the (attempted) change on the prototype change to cheerio in this file: https://github.com/jonkemp/inline-css/blob/4291cdf1ffbeddd6c11e963b5b3b5f6e35d5f4ae/lib/setTableAttrs.js

Is it possible achieve the same behavior without prototype modification?

For more info: I'm using node 14.13.1 and requiring inline-css like this:

let inline = require("inline-css");
mbaumgartl commented 2 years ago

Is it possible achieve the same behavior without prototype modification?

resetAttr was used inside the module only, so it's easy to replace:

diff --git a/lib/setTableAttrs.js b/lib/setTableAttrs.js
index 144056d..656191c 100644
--- a/lib/setTableAttrs.js
+++ b/lib/setTableAttrs.js
@@ -1,5 +1,3 @@
-const cheerio = require('cheerio');
-
 const tableStyleAttrMap = {
     table: {
         float: 'align',
@@ -53,20 +51,13 @@ const batchApplyStylesAsProps = ($el, sel, $) => {
     });
 };

-cheerio.prototype.resetAttr = function (attribute) {
-    if (!this.attr(attribute)) {
-        this.attr(attribute, 0);
-    }
-    return this;
-};
-
 module.exports = (el, $) => {
     let selector;
     let $el = $(el);

-    $el = $el.resetAttr('border')
-        .resetAttr('cellpadding')
-        .resetAttr('cellspacing');
+    [ 'border', 'cellpadding', 'cellspacing' ]
+        .filter((attribute) => !$el.attr(attribute))
+        .forEach((attribute) => $el.attr(attribute, 0));

     for (selector in tableStyleAttrMap) {
         if (selector === 'table') {

This should work with recent cheerio versions too. Unfortunately the HTML output changed and a lot of tests fail now.

Maybe it makes sense to pipe results through a beautifier or use something like chai-html.

@jonkemp: What do you think about necessary changes?