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

fix: lodash.pick vulnerability #244

Conversation

pedrokohler
Copy link
Collaborator

@pedrokohler pedrokohler commented Feb 9, 2024

Context

Solving high security issues:

https://github.com/ImagingDataCommons/Viewers/security/dependabot/198[High](https://github.com/ImagingDataCommons/Viewers/security/dependabot?q=is%3Aopen+severity%3Ahigh)
#198 opened 2 weeks ago • Detected in lodash.pick (npm) • yarn.lock

Refer to lodash/lodash#5809 (comment) to understand the reason for the new resolution to be a url and not a number.

Changes & Results

Testing

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] "OS:
  • [] "Node version:
  • [] "Browser:

Copy link
Collaborator

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try using a published version that contains the fix instead of archived versions.

@pedrokohler
Copy link
Collaborator Author

pedrokohler commented Feb 10, 2024

Please try using a published version that contains the fix instead of archived versions.

@igoroctaviano please refer to the link I posted in the description of the PR (this one: lodash/lodash#5809 (comment)) so you'll understand why it's not possible as of now to do what you ask.

Regards.

@igoroctaviano
Copy link
Collaborator

Can we use the second approach from that thread?

As @nclavaud notes, the modularised packages are now essentially deprecated. Thus, If you use lodash.pick or another modularised lodash subset as a direct dependency in your projects, drop it and replace it with lodash. If you're able to apply it to transitive uses, the solution suggested by @Pfahlf makes sense.

Maybe we just need to bump to the latest version of lodash and use lodash instead of modularized subset import.

@pedrokohler
Copy link
Collaborator Author

Can we use the second approach from that thread?

As @nclavaud notes, the modularised packages are now essentially deprecated. Thus, If you use lodash.pick or another modularised lodash subset as a direct dependency in your projects, drop it and replace it with lodash. If you're able to apply it to transitive uses, the solution suggested by @Pfahlf makes sense.

Maybe we just need to bump to the latest version of lodash and use lodash instead of modularized subset import.

@igoroctaviano I wouldn't know how to do that since we are not directly using lodash.pick, instead, the presence of lodash.pick comes from a long chain of dependencies from other packages:

@OHIF -> i18n -> locize-cli -> android-string-resource -> rdotjson -> cheerio -> lodash.pick

So I don't know how I'd force a package to use another lib instead of the one that's specified in that package's package.json, unless we do it the way I have done it, which is essentially doing that by telling it to download lodash from the url I provided, instead of downloading any version of lodash.pick.

FYI I also tried updating locize-cli to the latest version to see if it would help but it doesn't make a difference in this case.

Copy link
Collaborator

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@igoroctaviano igoroctaviano merged commit 855bc2b into ImagingDataCommons:master Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants