SAP / spartacus

Spartacus is a lean, Angular-based JavaScript storefront for SAP Commerce Cloud that communicates exclusively through the Commerce REST API.
Apache License 2.0
740 stars 386 forks source link

Glob-Parent Library Security Update. Bump Version to 5.1.2 #13025

Open giancorderoortiz opened 3 years ago

giancorderoortiz commented 3 years ago

See https://github.tools.sap/cx-commerce/spasec/issues/71

Platonn commented 3 years ago
@angular-devkit/build-angular@12.0.5 requires glob-parent@^3.1.0 via a transitive dependency on chokidar@2.1.8
  1. Impact is very very low. In the worst case it can slow down / crash the build of an angular app. See https://snyk.io/vuln/SNYK-JS-GLOBPARENT-1016905
  2. We can resolve this alert only when webpack-dev-server releases 4.0 and then angular must use this 4.0 as its dep. https://github.com/angular/angular-cli/issues/21214 This should happen in the following months https://github.com/webpack/webpack-dev-server/issues/3444#issuecomment-865513087
meeque commented 3 years ago

As discussed with @Platonn today, we could still try to use resolutions to bump this lib to version 5.1.2 or higher. This could be used to override versions for transitive uses of the glob-parent library. However, that would force some libraries that wanted version 3.1.0 to use a higher major instead.

That could certainly break stuff, but imho it's worth a shot. According to the changelog most changes are unlikely to affect Spartacus. If something breaks after all, build or tests will likely reveal that.

meeque commented 3 years ago

If it turns out that there is no feasible way to get rid of the vulnerable glob-parent version after all, let's have a closer look at the risk.

Dependabot classifies this as HIGH severity, but the underlying CVE-2020-28469 lists 2 alternative CVSS vectors:

I tend to agree with the second one, since the described ReDOS vulnerability will only have low availability impact.

Alternatively, we can evaluate the severity in the context of Spartacus. This vulnerable library is a devDependency of storefrontapp, which makes extremely unlikely to ever affect a production deployment of Spartacus. Any impact would be limited to workstations and CI servers of the Spartacus team. These lower availability requirements than a live Spartacus storefront. With that, I think the following CVSS environmental score is justified:

Imho that is a risk that Spartacus can accept for now. Eventually the transitive dependency on the vulnerable glob-parent version will disappear, as intermediate dependencies will adjust the version that they depend on.

meeque commented 3 years ago

Sorry, clicked the wrong button and dismissed the dependabot alert. Cannot find a way to undo it either. As said above, I'd be OK with keeping it dismissed.

But it would still be great to try upgrading to a non-vulnerable version of glob-parent, as soon as technically feasible.

giancorderoortiz commented 3 years ago

ok, will keep this ticket open until we fix or dismiss.

giancorderoortiz commented 3 years ago

To be handled after 4.0

giancorderoortiz commented 2 years ago

Requires discussion before 5.0 release.