Regex Denial of Service requires a semver major update to clean-css

[muxator]

Etherpad 1.7.5 uses clean-css@3.4.19, which contains a Regular Expression Denial of Service.
A fix would require updating to clean-css@4.2.1, as shown by npm audit:

$ npm audit
                                                                                
                       === npm audit security report ===                        
                                                                                
# Run  npm install clean-css@4.2.1  to resolve 1 vulnerability
SEMVER WARNING: Recommended action is a potentially breaking change
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ clean-css                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ clean-css                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ clean-css                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/785                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

[...]

The [documentation for clean-css 4.2.1 explains the breaking change]((https://github.com/jakubpawlowicz/clean-css/tree/v4.2.1#important-40-breaking-changes):

clean-css 4.0 introduces some breaking changes:

• root, relativeTo, and target options are replaced by a single rebaseTo option - this means that rebasing URLs and import inlining is much simpler but may not be (YMMV) as powerful as in 3.x;

And this change affects Etherpad:

etherpad-lite/src/node/utils/Minify.js

Line 418 in 357780d

new CleanCSS({relativeTo: base}).minify(content, function (errors, minified) {

[muxator]

Also, it is to be noted that (apart from the difficulty in making @import statements work in clean-css 4.x (see #3721 (comment)), the library is not actively maintained at the moment.

See this comment by the author, who is doing releases just because of kindness:

[...] The project is not actively maintained anymore and I'm looking into handing it over. [...]

Btw I can release the next version if there's a need.

Originally posted by @jakubpawlowicz in clean-css/clean-css#1105 (comment)

[jakubpawlowicz]

@muxator that's right, clean-css is not actively maintained anymore apart from occasional security bugfixes and PR from community.

[muxator]

Bumping clean-css from 3.4.19 to 4.2.3 required two changes.

The first one was more or less described in the documentation and in some Github isses. For the second one there was no other way than read the code and understand how to read a local file from an absolute path.

Probably this is worth a documentation PR on clean-css, even if it's out of support. No one has to suffer the same pain again. :)

Changes:

1. Disabling rebase was necessary because otherwise the URLs for the web fonts ended up being wrong;

   EXAMPLE 1:
   /static/css/src/static/font/fontawesome-etherpad.woff
   instead of
   /static/font/fontawesome-etherpad.woff

   EXAMPLE 2 (this is more surprising):
   /p/src/static/font/opendyslexic.otf
   instead of
   /static/font/opendyslexic.otf

2. CleanCSS.minify() can either receive a string containing the CSS, or an array of strings. When it receives a string, it is interpreted as the CSS code itself. If it receives an array, each array element is interpreted as an absolute local path from which the CSS file is read. I was not able to find any place in the documentation mentioning this, and it was necessary to study the source code.

Resource inlining was simplified in clean-css 4.x, but now we can no longer give a base path in relativeTo. For this reason, we had to ignore our already loaded content argument, and had to wrap the absolute path to the css file in an array and pass it to minify().

Gosh, this was hard to understand.