search

d3 / d3-color

Color spaces! RGB, HSL, Cubehelix, CIELAB, and more.
https://d3js.org/d3-color
ISC License
398 stars 91 forks source link

avoid backtracking #100

Closed mbostock closed 2 years ago

mbostock commented 2 years ago

Fixes #97. Supersedes #89 and #99. The problem was that this expression is fundamentally ambiguous:

/\s*([+-]?\d*\.?\d+(?:[eE][+-]?\d+)?)%\s*/

Since both the dot and the digits preceding the dot are optional, there’s a combinatorial explosion of possible valid matches. If we instead combine it into an optional group and make the dot required for that group, the explosion is avoided:

/\s*([+-]?(?:\d*\.)?\d+(?:[eE][+-]?\d+)?)%\s*/

Demo: https://observablehq.com/d/4b1d645fe3da1226

demartsc commented 2 years ago

Is it possible to apply this change to v1.4.1 of d3-color as well? For those who still need to support ES5 it would be helpful to have the vulnerability fixed on the ES5 compatible version of d3-color as well. I am happy to submit a PR off of the v1.4.1 tag if bumping that lesser version to v1.4.2 is something you are willing to do. Please let me know?

jayuen commented 1 year ago

@mbostock Same boat as @demartsc above. From https://github.com/d3/d3-color/pull/100, it looks like the change in the regex string would be compatible with the code in the 1.4.1 tag (I ran it locally and all the specs passed).

Would it be acceptable to get access to make a PR off the v1.4.1 tag for your review?

G-Rath commented 1 year ago

Would love to see a backport if possible; we would also appreciate if we could get a backport to the v2 tag as well because that's the highest major version supported by d3-interpolate and v3 of that package switches it to using ESM modules which we can't use in our applications.

I'm happy to help with these if there's anything I can do, including getting the GitHub advisories updated.

spreusler commented 1 year ago

@mbostock Same boat as @demartsc above. From #100, it looks like the change in the regex string would be compatible with the code in the 1.4.1 tag (I ran it locally and all the specs passed).

Would it be acceptable to get access to make a PR off the v1.4.1 tag for your review?

This would be great!

AtishayMsft commented 1 year ago

Would love to see a backport if possible; we would also appreciate if we could get a backport to the v2 tag as well because that's the highest major version supported by d3-interpolate and v3 of that package switches it to using ESM modules which we can't use in our applications.

I'm happy to help with these if there's anything I can do, including getting the GitHub advisories updated.

We would also like the dependencies of d3-color updated for v2 for d3-interpolate and d3-scale to support ES5 compatibility.

jayuen commented 1 year ago

Would love to see a backport if possible; we would also appreciate if we could get a backport to the v2 tag as well because that's the highest major version supported by d3-interpolate and v3 of that package switches it to using ESM modules which we can't use in our applications.

I'm happy to help with these if there's anything I can do, including getting the GitHub advisories updated.

@G-Rath and others who have 👍. I'm new to contributing to open source so would appreciate your help if possible. Since I don't have PR access on this repo, I forked it to jayuen/d3-color, created a branch off the v1.4.1 tag, and backported the fix there. I imagine we can create similar branches for the V2 and V3 tags. However, I'm not really sure what the next step is to a) silence the advisories and b) get this PR up for review in this repo. Any guidance would be appreciated!

G-Rath commented 1 year ago

@jayuen there's nothing more you can do for now - it's up to how @mbostock wants to handle that since no branch exists for those versions so there's nothing to PR into right now.

AtishayMsft commented 1 year ago

All, I have logged an issue with d3 for backporting this fix to v2.x at https://github.com/d3/d3-color/issues/108.