Fix dependency vulnerability d3-color < 3.1.0

[StanDeveloper]

• I have searched the issues of this repository and believe that this is not a duplicate.

What problem does this feature solve?

Updating the package will, fix the Redos vulnerability of the d3-color; GHSA-36jr-mh4h-2g58

What does the proposed API look like?

No api needed

[timges]

Wanted to post the same. Imo this should have highest priority as the vulnerability is of high severity. 👍🏼

[zavan]

I think #3009 solves this.

[timges]

Release 2.1.15 downgraded the version's again today. I guess dependabot's pr ( #3009 ) is not sufficient, as the checks are failing anyways

[vitalii-bulyzhyn]

Release 2.1.15 downgraded the version's again today. I guess dependabot's pr ( #3009 ) is not sufficient, as the checks are failing anyways

• 1 for this, still the same problem

[emirefek]

Are there any fix for this? What do you people suggest?
My dependabot and npm audit really going crazy about it.

[mark-todd]

I would say there are a couple of good options:

First is to increase versions of d3-interpolate and d3-scale - these are currently on old versions of these subpackages.

Failing that, finding an alternative to d3-interpolate and d3-scale (which may exist) - this could be a lot of work, but if recharts wants to be a viable software package this is absolutely essential to resolve.

[mark-todd]

Update: After some investigation I found a simple version increase on these d3 packages would do the trick! (Please see attached PR)

[mark-todd]

Dependabot also has a PR for this it turns out #3009

[G-Rath]

@mark-todd it's not that simple because v3 of d3-color is ESM based, meaning its not directly compatible with recharts - the upgrade you're suggesting has actually already been done and then reverted in v2.1.15 because it was breaking.

To resolve this properly one of three things needs to happen:

1. recharts does a new major version that switches it to using v3 of d3-color (via the other d3 dependencies)
   - recharts itself technically doesn't have to move to ESM, but it's still a breaking change because downstream consumers need to support ESM or transpile the modules
2. someone forks the related modules, publishes them to npm, and switches recharts over to using that
   - this would be a lot of work, and just be an overall annoyance; but maybe someone out there is able to spare the time?
3. d3-color releases a v2 with the fix
   - this would be the most optimal, though @mbostock has said they won't be doing this; I have asked them to reconsider but I'm not super hopeful

[mark-todd]

Hi @G-Rath - thanks for the detailed response! Option 1 looks pretty good to me - is there any reason not to make a major release for this? If a major security vulnerability doesn't qualify as a reason for a new major release - what does?

[G-Rath]

@mark-todd because it would require downstream consumers to be able to upgrade to that new major version, and in this case that would require being able to use ESM which is not an option for a lot people - so it would be just pushing the problem further downstream.

[mark-todd]

@G-Rath Hmm true - based on this comment: d3/d3-color#108 (comment) ...it seems like the v2 with fix version already exists. It won't be on npm, but it could be imported to Recharts with the git package syntax I suppose?

[G-Rath]

No, that is just a PR someone has made to the main branch (which is v3) asking that it be ported to v2 since a dedicated branch for v2 doesn't exist so there's no way to make a PR for it.

Using git-syntax would technically resolve the problem but that means npm has to do more complex work when installing (as it needs to use git to clone the repo, and that also requires authentication even ifs public) and would no longer be covered by security advisories so you'd not be able to detect if you're impacted by any future advisories for the package.

It's also not just that which'd bre required as recharts is not depending on d3-color directly - it's being pulled in by at least d3-interople so you'd actually need to either get the v2 of that to switch to using a different dependency (which seems unlikely to happen since they're not interested in doing a v2 backport for d3-color, which'd be less work than backporting for d3-interople), or fork that package and have that as a git dependency or publish it as a fork to the npm registry.

At this point unless @mbostock changes their mind, option 2 (forking a bunch of the d3 modules and publishing them to npm) is probably going to be the "best" option, if someone has bandwidth to maintain that.

(Having said that an alternative worth exploring first is if recharts can move off any of the d3 modules - even if only some of them can be replaced, it's probably going to make things easier)

[gergokee]

OK, so are there no plans for this ? Should we switch to another chart solution?

[mark-todd]

Yeah I agree with @gergokee on this - I know there are backwards compatibility concerns with the ESM part, but for people trying to start using recharts (and a lot of existing developers), this will mean we have to find other charting libraries. Apart from being a real shame for recharts, this will require a lot of overhaul for the existing developers - it effectively makes all of recharts a deprecated library.

[OhJayDubya]

@mark-todd So I am guessing that fixes like #3009 and #3022 aren't going to work due to the ESM issue? Just currently weighing up my options on whether to move off of recharts or hope these fixes will stop my repo from getting flagged for the d3-color vulnerability.

[mark-todd]

According to this: #3012 (comment) - unless recharts decides to make a major version increase (which I still think would be the right way to go) we don't have a lot of choice. Either that or someone publishes a fork of recharts to npm with this resolved I suppose

[wmhartl]

Agree with the posters above, recharts should bump its major version as a breaking change and move forward here. That's a common tactic in our ecosystem. We'd also like to stick with recharts versus moving to another package.

[kevbrowndev]

What's the best way folks have found to work around this issue while we wait for Recharts to be updated? Is replacing it with another library or a doctored version of Recharts really the best way to go?

[cfry2]

If you're using a bundler like Webpack or Rollup as well as using yarn you can add to your package.json

"resolutions": { "d3-color": "3.1.0" }

In order for this to play nice with Jest, add in your jest.config.js

transformIgnorePatterns: ['<rootDir>/node_modules/(?!(d3-interpolate|d3-color)/)'],

This assume's you're using babel to transpile your code

This works for me.

[kopach]

and if you're lucky to have pnpm as package manager, you can do like this for now:
in package.json add

  "pnpm": {
    "overrides": {
      "d3-interpolate@2.0.1>d3-color": "~3.1.0"
    }
  }

docs: https://pnpm.io/package_json#pnpmoverrides

[norbulcz]

Hello, First of all I appreciate what everybody has done with this library, I think is great. I just want to ask if is there an intention to fix this issue? Would be great to know if we can rely on this or try a workaround.

[kozzza]

What's the best workaround for npm users? I'm trying to override the d3-interpolate dependency, but am getting this error when installing recharts:

npm ERR! Override for d3-interpolate@^3.0.1 conflicts with direct dependency

Also can we get a timeline on the resolution of this dependency vulnerability? Thank you.

[jmfrancois]

This looks strange to me to go into the direction of a vendor packaged libraries from an external opensource project.

This article article already old https://devclass.com/2021/06/15/d3-7-0-goes-all-in-on-ecmascript-modules/ that explains why d3 has done the move to ESM.

Most of the tickets are about compatibility issue with tools which already support esm but need configuration.
Jest for example there is multiple way to configure it to work with esm. I gave an example PR here: #2915

Nextjs there is an example here: #2918

So why not just do the upgrade d3-* in major release ?

ESM is the future and tools should supports it more and more.

[ckifer]

People want a fix for the CVE, people want a patch that does not break their applications. 2.1.13/2.1.14 should've been released as 3.0 and we could've moved on from there.

Since we find ourselves where we do now the choices are 3.0 with the breaking change or this. This provides a CVE fix for those on 2.x.x that might not ever be able to use ESM (I know they should be able to, but this is the same reasoning as victory in their blogpost - we certainly have consumers who "can't")

We can still do the breaking change in v3, revert back to d3 rather than victory-vendor, and drop support for Node apps using cjs to continue the trudge forward towards ESM support. For now this seems a decent solution to prevent more people from needing to migrate charting libraries due to a security risk and to fix a pressing issue in 2.x.x.

This might not be the best action that could be taken but it is action and that's better than none. If there is a lot of pushback against this change I'd be happy to help push for a 3.0 release instead.

[ckifer]

Change is released in alpha version v2.3.0-alpha.1 (along with other small changes since 2.2). Tested briefly in next and with jest both with success. Tested in codesandbox with success.

https://www.npmjs.com/package/recharts/v/2.3.0-alpha.1

https://github.com/recharts/recharts/releases/tag/v2.3.0-alpha.1

Please help me test this so we can get 2.3.0 out in a week or so. Feel free to report here or tag me in issues with any bugs. Thanks all!

[mikegoatly]

@ckifer My use-case is fairly trivial, but it worked fine in my create-react-app based application

[nfiacco]

No problems here either! Just have simple bar and line charts though.

[wmhartl]

No problems here. Relatively straightforward responsive bar, line, and radar charts with default and custom labels, tooltips, and legends.

[esgem]

Apparently no problems here either. Simple graph with one or two bars per datapoint and according tooltips.

[firstclown]

I have a few charts:

• a fairly complex ComposedChart with Area, ReferenceAreas and an image based ReferenceDot with ChartLegend. Looks good to me. (and no errors or security warnings!) 👍
• a few horizontal BarCharts with ranges and Labels and XAxis and YAxis details 👍
• a section of Pie chart that we're using to show a gauge type visual 👍
• AreaChart with gradient Area underneath and tooltips 👍
• a Scatter with ReferenceLine with dots of different colors in ReferenceArea sections 👍

Looks great to me. I'd love to push this code live.

[ckifer]

Thanks for the responses all - we will most likely release sometime next week pending no bugs reported. Trying to take more precautions and do more testing than has been done in the past 🚀

[ckotyan]

@ckifer May be I am missing something here, After installing alpha version I still see the d3-color dependency on recharts when npm ls is run.

└─┬ recharts@2.3.0-alpha.1
└─┬ victory-vendor@36.6.8
└─┬ d3-interpolate@3.0.1
└── d3-color@1.4.1 deduped
Is that expected? my np audit fails still

[ckifer]

@ckotyan please try removing your node_modules, package-lock.json, and trying again. I can't reproduce that in an npm audit with 2.3.0-alpha.1

 npm ls d3-color

└─┬ recharts@2.3.0-alpha.1
  └─┬ victory-vendor@36.6.8
    └─┬ d3-interpolate@3.0.1
      └── d3-color@3.1.0

[ckifer]

Resolved in v2.3+ - https://github.com/recharts/recharts/releases/tag/v2.3.0

Let me know if anyone experiences issues. Thanks!

[andresebr]

The vulnerability issue has been fixed as mentioned, but after upgrading to v2.3.2, every page where a LineChart should be displayed crashes (see screenshots for errors). No issues when using v2.2.0.

[ckifer]

Any details on your setup @andresebr

Need a reproduction in order to address. Not failing anywhere we've tested.

Browser, framework if any, SSR or not, React version, TypeScript version

[andresebr]

Any details on your setup @andresebr

Need a reproduction in order to address. Not failing anywhere we've tested.

Browser, framework if any, SSR or not, React version, TypeScript version

Edge v107.0.1418.42, no framework, no SSR, React v18.2.0, typescript v4.9.4. Also, just in case, here's how the component that handles Line Charts looks:

export const LineChart: React.FunctionComponent<LineChartProps> = (
  props: LineChartProps
) => {
  return (
    <ResponsiveContainer width="90%" height={props.height}>
      <ReLineChart
        width={props.width}
        height={props.height}
        margin={{ top: 5, bottom: 5 }}
      >
        <CartesianGrid strokeDasharray="3 3" />
        <XAxis dataKey="x" type="category" allowDuplicatedCategory={false} />
        <YAxis dataKey="y" />
        <Tooltip />
        <Legend />
        {props.series.map((e, i) => (
          <Line
            dataKey="y"
            data={e.points}
            name={e.label}
            key={`Reference.${i}`}
            stroke={e.color}
          />
        ))}
        {props.references &&
          props.references.map((r) => (
            <ReferenceLine
              y={r.value}
              label={r.label}
              key={`Reference.${r.label}`}
              stroke={r.color}
              ifOverflow="extendDomain"
            />
          ))}
      </ReLineChart>
    </ResponsiveContainer>
  );
};

[ckifer]

@andresebr does it crash on other browsers?

[ckifer]

@andresebr https://codesandbox.io/s/recharts-crash-repro-144f85

I can't reproduce on CodeSandbox which makes me think a Browser issue with edge. Please confirm if your site works on other browsers. I can't test with edge.

Please file a new issue with this crash as well since this is closed

[andresebr]

@ckifer Same issue on other browsers. Tested on Firefox and Chrome. The CodeSandbox example works fine everywhere, though.

Now I'm wondering if it has something to do with my setup. It's a bit strange that there are no problems with v2.2.0 except for the vulnerability issues. I will file a new issue and include all the details that might be relevant. Thanks!

[nikolasrieble]

@andresebr Could you please for the sake of completeness include a toy dataset with which your problem surfaces?

[nikolasrieble]

mbostock/internmap#3

[nikolasrieble]

microsoft/TypeScript#10853