Security: gatsby-plugin-mdx is dependand on old version of trim, which has Regular Expression Denial of Service

[darkmatter18]

Preliminary Checks

• This issue is not a duplicate. Before opening a new issue, please search existing issues: https://github.com/gatsbyjs/gatsby/issues
• This issue is not a question, feature request, RFC, or anything other than a bug report. Please post those things in GitHub Discussions: https://github.com/gatsbyjs/gatsby/discussions

Summary

`-- gatsby-plugin-mdx@2.13.0
  `-- remark@10.0.1
    `-- remark-parse@6.0.3
      `-- trim@0.0.1 

gatsby-plugin-mdx is dependant upon remark, which has a dependency of trim@0.0.1. Trim0.0.1 has a High level vulnerability, as mentioned in https://www.npmjs.com/advisories/1700.

Steps to Resolve this Issue

So the update of remark is needed in dependency, to make sure that the plugin stays vulnerability free

[LekoArts]

Hi, thanks for the issue!

While those audit messages might seem scary they are almost always irrelevant in the context of Gatsby as it's a build tool. So there is nothing to "fix" as the warning is not relevant.

Gatsby creates static assets and runs everything at build time and not during runtime. npm audit is designed for runtime / Node apps so it flags issues that can occur there. This means that almost every "vulnerability" report we receive are false positives. While in principle Gatsby can also have vulnerabilities you need to make sure that it's relevant to Gatsby before reporting it. For example a "Regex DDOS attack" can never be a real vulnerability for a development-time tool. If you want to override/update a transitive dependency you can use yarn resolutions.

If you know that a vulnerability affects Gatsby because you understand what the vulnerability is, please report it here. Thanks!

[wesrice]

@LekoArts I get what you're saying, but I think it might be beneficial to look at this particular issue a bit closer.

Consider the details of this issue.

The entire process of testing (the regex) against a 30 characters long string takes around ~52ms. But when given an invalid string, it takes nearly two seconds to complete the test, over ten times as long as it took to test a valid string. The dramatic difference is due to the way regular expressions get evaluated.

remark-parse is using trim() in a process that:

// Removes the minimum indent from every line in value. Supports both tab,
// spaced, and mixed indentation (as well as possible).

Given the nature of how many times trim() might be executed with a false result in this process, on the surface it seems that this issue could potentially impact performance during build time, potentially with gatsby unwittingly acting as the attacker of the build machine.

Perhaps I'm misunderstanding the issue at hand. Regardless, I think it's worth a look by the Gatsby team just to be safe.

[MangelMaxime]

I would also like to have the security warning fixed, some of my library users raised concern about this situation too.

When installing your dependencies this is never a good sign to see warning and some company can refuse to use a library because of that...

Edit:

It is also worth noting that if we move to yarn to benefit from "yarn resolutions" feature as proposed then we don't get a warning about the vulnerability just by switching to yarn itself.

In the screenshot above, you can see that npm audit report several vulnerabilities while running yarn audit returns none...

This means that it would not be easy to know if we have a vulnerability or not in the dependencies.