Unpatched path-to-regexp ReDoS in 0.1.x (CVE-2024-52798, CVE-2024-45296)

[dbas-dn]

CVE-2024-52798
CVE-2024-45296

A new vulnerability has appeared in a dependent package.
When forcibly updating to version 0.1.12, an error occurs: TypeError: pathRegexp is not a function.

[dbas-dn]

I’ve noticed an issue with how pnpm handles automatic vulnerability fixes. When a package version is updated, pnpm uses the >= prefix, for example:

"path-to-regexp@<0.1.12": ">=0.1.12"

This allows pnpm to install any version starting from 0.1.12 and above, including newer major versions like 3.3.0. However, this can lead to compatibility issues if the package consuming the dependency is not prepared for a major version upgrade.
To avoid this problem, I suggest replacing >= with either:

• ^ to limit the updates within the same major version, or
• <1.0.0 if the dependency needs to stay within the same major version range.
  For example:

"path-to-regexp": ">=0.1.12 <1.0.0"

This would ensure that the installed version stays below 1.0.0, preventing unexpected major version upgrades while still addressing vulnerabilities.

[dbas-dn]

In any case, the version of the dependent package should be updated. There are no changes that could break the express package:
Compare versions v0.1.10 & v0.1.12

[viceice]

updating path-to-regexp to v0.1.12 causes some weired invalid rexex errors here

https://github.com/verdaccio/verdaccio/blob/b332ba15c9b48e0e4760a7bc5e55db63c9d833d9/src/api/web/api/package.ts#L117

Will copy stack trace later

[viceice]

here's the error:

uncaught exception, please report this
SyntaxError: Invalid regular expression: /^/-/verdaccio/data/package/readme/(?:@(?:((?:(?!/|/-/verdaccio/data/package/readme/@).)+?))/)?(?:((?:(?!/|/-/verdaccio/data/package/readme/@/)?).)+?))(?:/([^/]+?))?/?$/i: Unmatched ')'
    at new RegExp (<anonymous>)
    at pathToRegexp (/opt/verdaccio/node_modules/.pnpm/[email protected]/node_modules/path-to-regexp/index.js:155:10)
    at new Layer (/opt/verdaccio/node_modules/.pnpm/[email protected]/node_modules/express/lib/router/layer.js:45:17)
    at Function.route (/opt/verdaccio/node_modules/.pnpm/[email protected]/node_modules/express/lib/router/index.js:505:15)
    at proto.<computed> [as get] (/opt/verdaccio/node_modules/.pnpm/[email protected]/node_modules/express/lib/router/index.js:520:22)
    at addPackageWebApi (/opt/verdaccio/node_modules/.pnpm/[email protected][email protected]/node_modules/verdaccio/build/api/web/api/package.js:96:13)
    at _default (/opt/verdaccio/node_modules/.pnpm/[email protected][email protected]/node_modules/verdaccio/build/api/web/api/index.js:28:24)
    at _default (/opt/verdaccio/node_modules/.pnpm/[email protected][email protected]/node_modules/verdaccio/build/api/web/index.js:34:31)
    at defineAPI (/opt/verdaccio/node_modules/.pnpm/[email protected][email protected]/node_modules/verdaccio/build/api/index.js:95:30)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async _default (/opt/verdaccio/node_modules/.pnpm/[email protected][email protected]/node_modules/verdaccio/build/api/index.js:122:10)

[bjohansebas]

Hi @dbas-dn, could you let us know the version of Express you are using? Version 4.21.2 was recently released, which includes an update to the dependency and addresses the vulnerability

[ctcpip]

in v4, path-to-regexp is actually pinned: "path-to-regexp": "0.1.12"

anyway, this was patched in 4.21.2

[blakeembrey]

@viceice I would recommend opening that as a new issue, but unfortunately in the process of trying to fix the CVE in V4 I had to sacrifice some previously valid paths due to ambiguity in the [email protected] paths. To solve your specific problem immediately I would recommend switching to path array feature of Express, so it would look like:

['/-/verdaccio/data/package/readme/@:scope/:package/:version?', '/-/verdaccio/data/package/readme/:package/:version?']

This isn't ideal, but the way the backtracking protection has been written is that it assumed everything prior to the parameter was regular characters, while clearly ( in this case is not. I think we could discard ( and ) (when un-escaped) without any ill side effect but other special characters will have the same issue.

[Pyker]

This honestly just resulted in an outage for me, with the same error as #6216 (comment). I wasn't expecting a patch release to be a breaking change that causes the entire application to collapse without any changes to the code.