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

frontend-build: High security vulnerability needs semver-regex update (via image-webpack-loader) #107

Closed
1 task
dianekaplan opened this issue May 31, 2022 · 10 comments · Fixed by openedx/frontend-build#259
Assignees

Comments

@dianekaplan
Copy link

dianekaplan commented May 31, 2022

High: Regular Expression Denial of Service (ReDOS)
Patched in semver-regex version >=3.1.3
dependency chain: @edx/frontend-build > image-webpack-loader > imagemin-gifsicle > gifsicle > bin-wrapper > bin-version-check > bin-version > find-versions > semver-regex
more info: GHSA-44c6-4v22-4mhx

It looks like frontend-build currently uses image-webpack-loader 8.1.0, which only uses semver-regex version 2.0.0. (We need to update image-webpack-loader to a version that uses semver-regex >=3.1.3).

PRs

@mamankhan99
Copy link

mamankhan99 commented Jun 2, 2022

Hi @dianekaplan, 8.1.0 is the latest version of image-webpack-loader, we can not update it at the moment. Should we wait for the patch? I would appreciate your suggestions.

@mamankhan99 mamankhan99 moved this from Todo to In Code Review in FED-BOM Jun 8, 2022
@mamankhan99 mamankhan99 moved this from In Code Review to In Progress in FED-BOM Jun 8, 2022
@mamankhan99 mamankhan99 moved this from In Progress to Blocked in FED-BOM Jun 8, 2022
@jmbowman
Copy link

It looks like the issue has been flagged upstream but there has not yet been a response: tcoopman/image-webpack-loader#414 . Could you go ahead and create a PR upstream to address it?

@jmbowman
Copy link

@mamankhan99 Are you (or anybody else on FED-BOM) up for creating an upstream PR as suggested above?

@mamankhan99
Copy link

We can but we will need to create PRs from bin-version to image-webpack-loader to fix the issue. Should we proceed?

@jmbowman
Copy link

jmbowman commented Jul 6, 2022

Sorry for not seeing this earlier. I'm ok with that, but is there an alternative we can use instead of image-webpack-loader? The webpack docs mention image-minimizer-webpack-plugin , is that the preferred library for this functionality now? Is it in a better state of maintenance? There's some relevant discussion in webpack-contrib/image-minimizer-webpack-plugin#225 .

@mamankhan99
Copy link

Yes! we can replacte it with image-minimizer-webpack-plugin. Let me discuss the feasibility with my team and then we can start working on it.

@mamankhan99 mamankhan99 moved this from Blocked to Todo in FED-BOM Jul 15, 2022
@BilalQamar95 BilalQamar95 moved this from Todo to In Progress in FED-BOM Aug 29, 2022
@BilalQamar95
Copy link

BilalQamar95 commented Aug 29, 2022

We are using v8.1.0 of image-webpack-loader which is the latest version. The package is bundled with following optimizers
imagemin-gifsicle: Imagemin plugin for Gifsicle (Compress GIF images)
imagemin-mozjpeg: Imagemin plugin for mozjpeg (Compress JPEG images)
image-pngquant: Imagemin plugin for pngquant (Compress PNG images)

Dependency chain for these optimizers is as follow:
imagemin-gifsicle > gifsicle > bin-wrapper > bin-version-check > bin-version > find-versions > semver-regex
imagemin-mozjpeg > mozjpeg > bin-wrapper > bin-version-check > bin-version > find-versions > semver-regex
iimagemin-pngquant> pngquant-bin > bin-wrapper > bin-version-check > bin-version > find-versions > semver-regex

All of them have dependency on bin-wrapper. The latest version of bin-wrapper is using bin-version-check: 4.0.0 which is using semver-regex: 2.0.
The issues lies with (latest version of) bin-wrapper using outdated bin-version-check. (bin-wrapper PR updating bin-version-check to latest version, bumping semver-regex to v4.

We can replace image-webpack-loader with image-minimizer-webpack-plugin, as long as we also move away from imagemin
imagemin - optimize your images by default, since it is stable and works with all types of images
squoosh - while working in experimental mode with .jpg, .jpeg, .png, .webp, .avif file types.
sharp - High performance Node.js image processing, to resize and compress JPEG, PNG, WebP, AVIF and TIFF images. Uses the libvips library.

Since imagemin is depending on aforementioned plugins(gifsicle, mozjpeg, pngquant), we would have to consider squoosh or sharp in order to avoid ReDOS. In case of squoosh there seems to be no support for GIF.

@arbrandes
Copy link

@BilalQamar95, @jmbowman, are we moving forward with image-minimizer-webpack-plugin? For what it's worth, sharp looks nice and unbloated.

@BilalQamar95
Copy link

BilalQamar95 commented Nov 1, 2022

Replaced image-webpack-loader with image-minimizer-webpack-plugin openedx/frontend-build#259

@arbrandes The shift would be required to resolve ReDOS, a High security vulnerability due to semver-regex. The PR for it is waiting on a re-review from Adam

@arbrandes
Copy link

@BilalQamar95, thanks! I hadn't noticed the PR existed.

Repository owner moved this from In Progress to Done in FED-BOM Nov 22, 2022
@arbrandes arbrandes moved this from In progress to Closed in Frontend Working Group Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

6 participants