es-toolkit icon indicating copy to clipboard operation
es-toolkit copied to clipboard

fix: Fix prototype pollution issue in es-toolkit

Open raon0211 opened this issue 10 months ago • 8 comments

Summary by CodeRabbit

  • Bug Fixes
    • Improved object manipulation functions to prevent unsafe property access, updates, merges, or deletions (such as __proto__), enhancing security against prototype pollution vulnerabilities.
  • Tests
    • Added comprehensive tests to ensure object utility functions do not allow prototype pollution via unsafe property keys.
  • Documentation
    • Removed documentation files for the flatMap function in multiple languages.

raon0211 avatar Apr 24 '25 08:04 raon0211

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
es-toolkit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2025 6:48am

vercel[bot] avatar Apr 24 '25 08:04 vercel[bot]

Codecov Report

Attention: Patch coverage is 54.54545% with 10 lines in your changes missing coverage. Please review.

Project coverage is 99.28%. Comparing base (d39d385) to head (7fc81b5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1151      +/-   ##
==========================================
- Coverage   99.51%   99.28%   -0.23%     
==========================================
  Files         457      458       +1     
  Lines        4339     4361      +22     
  Branches     1286     1296      +10     
==========================================
+ Hits         4318     4330      +12     
- Misses         21       31      +10     
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov-commenter avatar Apr 24 '25 09:04 codecov-commenter

@coderabbitai review

raon0211 avatar May 04 '25 10:05 raon0211

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar May 04 '25 10:05 coderabbitai[bot]

Error: Could not generate a valid Mermaid diagram after multiple attempts.


📜 Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f76be6accbb4b26174697e19cf0d2cf3eb9dab2 and 06f7b9fde166c4643d319b7b2bfc8c740bf465ae.

📒 Files selected for processing (12)
  • docs/ja/reference/compat/array/flatMap.md (0 hunks)
  • docs/ko/reference/compat/array/flatMap.md (0 hunks)
  • docs/reference/compat/array/flatMap.md (0 hunks)
  • docs/zh_hans/reference/compat/array/flatMap.md (0 hunks)
  • src/_internal/isUnsafeProperty.ts (1 hunks)
  • src/compat/object/get.ts (4 hunks)
  • src/compat/object/mergeWith.ts (2 hunks)
  • src/compat/object/unset.ts (4 hunks)
  • src/compat/object/updateWith.ts (2 hunks)
  • src/object/merge.ts (2 hunks)
  • src/object/mergeWith.ts (2 hunks)
  • tests/__proto__.spec.ts (1 hunks)
💤 Files with no reviewable changes (4)
  • docs/reference/compat/array/flatMap.md
  • docs/zh_hans/reference/compat/array/flatMap.md
  • docs/ko/reference/compat/array/flatMap.md
  • docs/ja/reference/compat/array/flatMap.md
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/compat/object/mergeWith.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
src/object/mergeWith.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
src/object/merge.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
src/compat/object/unset.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
src/compat/object/get.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
tests/__proto__.spec.ts (3)
src/object/merge.ts (1)
  • merge (44-78)
src/object/mergeWith.ts (1)
  • mergeWith (52-83)
src/compat/object/unset.ts (1)
  • unset (24-79)
src/compat/object/updateWith.ts (1)
src/_internal/isUnsafeProperty.ts (1)
  • isUnsafeProperty (11-13)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: codecov
🔇 Additional comments (18)
src/_internal/isUnsafeProperty.ts (1)

1-13: Well-structured security utility function

This function provides a centralized way to identify unsafe property keys that could lead to prototype pollution vulnerabilities. Currently, it only checks for __proto__, which is the most common vector for prototype pollution attacks.

The documentation clearly explains the purpose and importance of this security measure.

src/object/mergeWith.ts (2)

1-1: Good security enhancement with import of isUnsafeProperty

Adding this import properly sets up the security check for the mergeWith function.


62-65: Strong security improvement to prevent prototype pollution

This check correctly guards against prototype pollution attacks by skipping any unsafe properties (like __proto__) during the merge operation. The implementation is simple and effective - it sits early in the loop to prevent any processing of unsafe keys.

src/compat/object/mergeWith.ts (2)

2-2: Appropriate import for security utility

Good addition of the import for isUnsafeProperty to enhance security.


416-419: Effective guard against prototype pollution

This security check correctly prevents prototype pollution attacks by skipping the merging of unsafe properties. The implementation follows a consistent pattern with other object manipulation functions in the codebase.

src/object/merge.ts (2)

1-1: Good security enhancement with import

Adding this import properly sets up the prototype pollution prevention mechanism.


53-56: Critical security protection implemented correctly

This check effectively guards against prototype pollution vulnerabilities by preventing the merging of __proto__ or other potentially unsafe properties. The implementation is consistent with the pattern used in other object manipulation functions.

src/compat/object/updateWith.ts (2)

1-1: Good addition of the security utility.

The import for isUnsafeProperty is appropriate for the security enhancement being implemented.


43-45: Excellent security enhancement to prevent prototype pollution.

This code correctly implements a guard clause to skip processing unsafe property keys like __proto__, which prevents prototype pollution attacks. This is an important security fix that ensures the function can't be abused to modify the prototype chain.

Adding this check before any processing or assignment of the key ensures the function won't inadvertently modify prototypes even through deeply nested paths.

src/compat/object/get.ts (4)

2-2: Good addition of the security utility.

The import for isUnsafeProperty is appropriate for the security enhancement being implemented.


319-321: Effective security check for string paths.

This guard clause correctly prevents access to unsafe properties like __proto__ when the path is a string, returning the default value instead. This is a key defense against prototype pollution.


360-362: Good protection for converted path strings.

This check handles the case where the path is converted to a string, ensuring consistent security regardless of the original path type.


387-389: Important protection for array paths.

This check in the getWithPath function ensures that even when accessing nested properties through an array path, unsafe properties like __proto__ are still protected, providing comprehensive security.

src/compat/object/unset.ts (4)

2-2: Good addition of the security utility.

The import for isUnsafeProperty is appropriate for the security enhancement being implemented.


47-49: Effective security check for non-string paths.

This guard clause prevents deletion of unsafe properties like __proto__ when using symbol, number, or object paths, returning false to indicate the operation wasn't performed.


67-69: Good protection for string paths.

This check prevents deletion of unsafe properties when using string paths, maintaining a consistent security approach throughout the function.


89-91: Critical protection in the helper function.

This check in unsetWithPath ensures that even when working with path arrays, the function won't delete unsafe properties like __proto__. This is crucial as it prevents prototype pollution at the deepest execution level.

tests/__proto__.spec.ts (1)

1-76: Excellent comprehensive test suite for prototype pollution protection.

This test suite thoroughly verifies that all the object manipulation functions (merge, mergeWith, toMerged, set, unset, zipObjectDeep) properly protect against prototype pollution. The tests check both the main library and compatibility layer implementations.

Each test correctly:

  1. Attempts to pollute the prototype via __proto__
  2. Verifies that the pollution didn't occur by checking that {}.polluted is undefined
  3. For unset, verifies that it doesn't remove built-in prototype methods

These tests provide strong assurance that the prototype pollution fixes work as intended across the entire library.

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar May 04 '25 10:05 coderabbitai[bot]

@coderabbitai review

raon0211 avatar May 04 '25 10:05 raon0211

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar May 04 '25 10:05 coderabbitai[bot]

@raon0211 Hello, I noticed this PR.

In general, the denylist for prototype pollution should contain __proto__, constructor, and prototype.

  • E.g. The patched commit for GHSA-624g-8qjg-8qxf is:
    • https://github.com/edmundhung/conform/commit/4819d51b5a53fd5486fc85c17cdc148eb160e3de

The reason is:

console.log({}.constructor.prototype === {}.__proto__); // -> true

({}).constructor.prototype.polluted = "yes";
console.log({}.polluted); // -> yes

arkark avatar May 09 '25 06:05 arkark