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

security(visx/scale): Update for d3-color dependencies? #1577

Closed
eelco opened this issue Sep 29, 2022 · 11 comments · Fixed by #1578 or #1621
Closed

security(visx/scale): Update for d3-color dependencies? #1577

eelco opened this issue Sep 29, 2022 · 11 comments · Fixed by #1578 or #1621

Comments

@eelco
Copy link

eelco commented Sep 29, 2022

Given this vulnerability GHSA-36jr-mh4h-2g58, it would be great to get an update of all visx packages that directly and indirectly use d3-color.

@williaster williaster changed the title Update for d3-color dependencies? security(visx/*): Update for d3-color dependencies? Sep 29, 2022
@williaster williaster changed the title security(visx/*): Update for d3-color dependencies? security(visx/scale): Update for d3-color dependencies? Sep 29, 2022
@williaster
Copy link
Collaborator

thanks for opening this. we don't have any direct dependencies on d3-color, but yarn why d3-color gives

=> Found "@visx/scale#d3-color@2.0.0"
info Reasons this module exists
   - "_project_#@visx#scale#d3-scale#d3-interpolate" depends on it

This affects d3-color versions <3.1.0. The yarn.lock file in our repo shouldn't affect consumers necessarily, they should be able to force resolve that to whatever they want. Seems like the best we could do is bump d3-scale in @visx/scale. But d3-scale still specifies "d3-interpolate": "1.2.0 - 3" and d3-interpolate still specifies "d3-color": "1 - 3" so even that wouldn't guarantee a fix until those packages are updated.

@rowanlend
Copy link

hmm - any updates on this front? new project wants to use @visx/scale, but dependency graph points to d3-interpolate: 1.4.0 --> d3-color: 1.4.1 --> high vulnerability causing builds to fail

i'm not sure if d3-interporlate > 1.4.0 addresses the issue, but Mike Bostock closed the related issue here: d3/d3-interpolate#106

@collinwu
Copy link

Unfortunately this was a blocker. We love the visx library, but we had to replace visx/scale with d3-scale. I didn't look at the visx/scale source code too closely. I assumed it's primarily a wrapper around d3-scale so we dropped in d3-scale as a replacement, updated the syntax and it's working fine for us currently. Could be an option if your pipelines are failing builds due to this.

from:

import { scaleLinear } from "@visx/scale";

const yScale = scaleLinear<number>({
  domain: [0, maxNumber])
});

to:

import { scaleLinear } from "d3-scale";

const yScale = scaleLinear<number>().domain([0, maxNumber]);
$ npm install d3-scale --save

added 8 packages, and audited 757 packages in 3s

133 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

also if typescript make sure you grab the type defs from @types/d3-scale

Cheers

@eelco
Copy link
Author

eelco commented Nov 27, 2022

It’s not pretty, but we solved it by overriding d3-color’s version with Yarn’s resolutions in package.json (npm has overrides for that).

@JayWelsh
Copy link
Contributor

@eelco would you mind sharing a little more details about what you did? 🥇

@JayWelsh
Copy link
Contributor

Unfortunately, @visx/axis, @visx/brush, @visx/grid, @visx/shape (likely others, too) depend on a vulnerable version of @visx/scale & @visx/shape (if using any of these then it doesn't resolve the issue to replace @visx/scale with d3-scale).

In case it is helpful:

Screenshot 2022-12-16 at 04 37 53

@haydn
Copy link

haydn commented Dec 16, 2022

@JayWelsh You can use Yarn's selective resolutions like this:

image

@JayWelsh
Copy link
Contributor

JayWelsh commented Dec 16, 2022

Fantastic temporary workaround, thanks a lot for sharing! 💯

@JayWelsh
Copy link
Contributor

JayWelsh commented Dec 16, 2022

It looks like Recharts might be leaning towards making use of victory-vendor which was created by FormidableLabs for their victory charting library.

victory-vendor blog post (discusses motivation behind this and introduction of victory-vendor package):

https://formidable.com/blog/2022/victory-esm/

victory-vendor package README:

https://github.com/FormidableLabs/victory/blob/main/packages/victory-vendor/README.md

Github thread for history on discussion which ultimately lead to development of victory-vendor:

FormidableLabs/victory#2124 (comment)

Evidence of Recharts discussion beginning to lean this way:

recharts/recharts#3012 (comment)

If visx also decides to make use of victory-vendor package directly I think it could be an overall win.

There's no doubt that this is a problem which should be addressed because newcomers to visx as well as seasoned visx users are not going to be comfortable with the current vulns included in the default (up to date) install.

JayWelsh referenced this issue in JayWelsh/silo-observer-frontend Dec 17, 2022
@JayWelsh
Copy link
Contributor

P.S. for anyone in this thread looking for an update, our knight in shining armour @williaster has set the wheels in motion to migrate away from nimbus and resolve this problem: #1609 🥇

williaster pushed a commit that referenced this issue Dec 22, 2022
* deps(scale): bump `d3-interpolate` and `d3-scale`

Closes #1577

Bumping these packages allows for `d3-color` version 3.1.0 or higher to
be installed, fixing this vulnerability:
GHSA-36jr-mh4h-2g58

* deps(scale): update yarn.lock

* chore: configure modules that need to be transformed by babel
@williaster
Copy link
Collaborator

noting that the fix was reverted to enable release as a breaking change due to the ESM-only d3 modules. going to reopen until #1621 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants