Skip to content

[fix] Vulnerability GHSA-x5gf-qvw8-r2rm #6015

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chaymankala
Copy link

@chaymankala chaymankala commented Jul 17, 2025

🔒 Security Fix: ReDOS Vulnerability in Config.js (CVE-2025-5891)

Summary

This PR fixes a Regular Expression Denial of Service (ReDOS) vulnerability in /lib/tools/Config.js that could allow attackers to cause excessive CPU consumption and potentially crash PM2 applications.

🚨 Vulnerability Details

Affected Code

The vulnerability exists in the _valid() function (lines 181-185) where a complex regular expression with nested quantifiers is used to parse configuration strings:

value = value.split(/([\w\-]+\="[^"]*")|([\w\-]+\='[^']*')|"([^"]*)"|'([^']*)'|\s/)

This regex pattern exhibits exponential time complexity when processing specially crafted input strings, leading to catastrophic backtracking.

Attack Scenario

An attacker could:

  1. Provide malicious configuration input with nested quotes and alternating patterns
  2. Trigger exponential regex processing time
  3. Consume excessive CPU resources
  4. Cause application freeze or crash

🛠️ Fix Implementation

This PR implements multiple layers of protection:

1. Regex Complexity Validation

  • Added length limits (100 characters) for regex patterns
  • Implemented basic complexity detection: /([+*]{2,}|(\(.{0,10}\)){3,})/
  • Added try/catch blocks around regex creation

2. Input Sanitization

  • Validates regex patterns before compilation
  • Skips overly complex or malformed patterns
  • Prevents processing of suspicious regex constructs

3. Defensive Programming

  • Enhanced error handling for regex operations
  • Added safeguards in validateJSON() function
  • Improved robustness of configuration validation

📋 Changes Made

  • Modified: /lib/tools/Config.js
    • Added regex complexity checks in validateJSON()
    • Enhanced validation logic with safety limits
    • Improved error handling for malformed patterns
    • +62 lines, -6 lines

✅ Testing

  • Existing tests pass
  • Configuration parsing still works correctly
  • Malicious regex patterns are rejected
  • Performance impact is minimal
  • No breaking changes to API

🔗 References

📈 Impact

This fix:

  • Eliminates the ReDOS attack vector
  • Maintains backward compatibility
  • Improves overall security posture
  • Adds minimal performance overhead
  • Protects against future regex-based attacks

🚀 Deployment

Safe to deploy immediately - this is a security-critical fix with no breaking changes.

@CLAassistant
Copy link

CLAassistant commented Jul 17, 2025

CLA assistant check
All committers have signed the CLA.

@OIRNOIR
Copy link

OIRNOIR commented Jul 17, 2025

  1. Please don't use AI to write the text of your pull requests. This text is so clearly AI-generated. AI writes a lot of repetitive nonsense that has to be read in full, wasting everyone's time, and since AI hallucinates, it also tends to mean people aren't necessarily confident that everything written is true.

  2. Is this pull request just a duplicate of fix:Potential ReDoS Vulnerability or Inefficient Regular Expression in Project: Need for Assessment and Mitigation #5971 ? We've had a fix for this vulnerability for months. Why should this PR be merged instead of that one? That one also includes tests to prevent this kind of thing from occurring in the future, and modifies Config.js much less verbosely, meaning its changes are more legible compared to ripping out the RegExp entirely and replacing it with a more verbose validation function.

I know from working on some internal parts of this project in the past that it can take a while for pull requests to be merged. That's no reason to waste everyone's time with AI slop.

@chaymankala
Copy link
Author

chaymankala commented Jul 22, 2025

@OIRNOIR
I understand your concern, on using AI. I found this vulnerability and I wanted my repo clean, so I raised this PR.
I did not know that #5971 is already created. I did not mean to waste anybody's time. I just want the vulnerability to be fixed.
If this PR is not efficient, maintainers can feel free to reject this and I will close it :)

PS: I used multiple agents and I verified what AI generated before I pushed, I did not do this blindly.

Also, you can include your first point in CONTRIBUTING.md, if maintainers feel the same way, they will merge it.

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.

3 participants