diff --git a/packages/astro/src/server/meta.ts b/packages/astro/src/server/meta.ts index 7f1f544a19e6..643565e7afaa 100644 --- a/packages/astro/src/server/meta.ts +++ b/packages/astro/src/server/meta.ts @@ -73,6 +73,7 @@ export function isValidBaggageString(baggage?: string): boolean { const keyRegex = "[-!#$%&'*+.^_`|~A-Za-z0-9]+"; const valueRegex = '[!#-+-./0-9:<=>?@A-Z\\[\\]a-z{-}]+'; const spaces = '\\s*'; + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp for readability, no user input const baggageRegex = new RegExp( `^${keyRegex}${spaces}=${spaces}${valueRegex}(${spaces},${spaces}${keyRegex}${spaces}=${spaces}${valueRegex})*$`, ); diff --git a/packages/eslint-config-sdk/src/base.js b/packages/eslint-config-sdk/src/base.js index 70f54626fbc7..2b98cb6038dd 100644 --- a/packages/eslint-config-sdk/src/base.js +++ b/packages/eslint-config-sdk/src/base.js @@ -135,6 +135,11 @@ module.exports = { '@sentry-internal/sdk/no-optional-chaining': 'error', '@sentry-internal/sdk/no-nullish-coalescing': 'error', + // We want to avoid using the RegExp constructor as it can lead to invalid or dangerous regular expressions + // if end user input is used in the constructor. It's fine to ignore this rule if it is safe to use the RegExp. + // However, we want to flag each use case so that we're aware of the potential danger. + '@sentry-internal/sdk/no-regexp-constructor': 'error', + // JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation, // even if it may seems excessive at times, is important to emphasize. Turned off in tests. 'jsdoc/require-jsdoc': [ diff --git a/packages/eslint-plugin-sdk/src/index.js b/packages/eslint-plugin-sdk/src/index.js index 2798a005c277..c03a042ca836 100644 --- a/packages/eslint-plugin-sdk/src/index.js +++ b/packages/eslint-plugin-sdk/src/index.js @@ -15,5 +15,6 @@ module.exports = { 'no-eq-empty': require('./rules/no-eq-empty'), 'no-unsupported-es6-methods': require('./rules/no-unsupported-es6-methods'), 'no-class-field-initializers': require('./rules/no-class-field-initializers'), + 'no-regexp-constructor': require('./rules/no-regexp-constructor'), }, }; diff --git a/packages/eslint-plugin-sdk/src/rules/no-regexp-constructor.js b/packages/eslint-plugin-sdk/src/rules/no-regexp-constructor.js new file mode 100644 index 000000000000..f08cb68504ef --- /dev/null +++ b/packages/eslint-plugin-sdk/src/rules/no-regexp-constructor.js @@ -0,0 +1,29 @@ +'use strict'; + +/** + * This rule was created to flag usages of the RegExp constructor. + * It is fine to use `new RegExp` if it is necessary and safe to do so. + * "safe" means, that the regular expression is NOT created from unchecked or unescaped user input. + * Avoid creating regular expressions from user input, because it can lead to invalid expressions or vulnerabilities. + */ +module.exports = { + meta: { + docs: { + description: + "Avoid using `new RegExp` constructor to ensure we don't accidentally create invalid or dangerous regular expressions from user input.", + }, + schema: [], + }, + create: function (context) { + return { + NewExpression(node) { + if (node.callee.type === 'Identifier' && node.callee.name === 'RegExp') { + context.report({ + node, + message: 'Avoid using the RegExp() constructor with unsafe user input.', + }); + } + }, + }; + }, +}; diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index fae0624e7005..1e31ffaaef0c 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -252,6 +252,7 @@ function convertNextRouteToRegExp(route: string): RegExp { ) .join('/'); + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input return new RegExp( `^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end ); diff --git a/packages/nextjs/src/config/loaders/prefixLoader.ts b/packages/nextjs/src/config/loaders/prefixLoader.ts index 3077b3f70db0..a9ca78667b53 100644 --- a/packages/nextjs/src/config/loaders/prefixLoader.ts +++ b/packages/nextjs/src/config/loaders/prefixLoader.ts @@ -30,6 +30,7 @@ export default function prefixLoader(this: LoaderThis, userCode: // Fill in placeholders let templateCode = fs.readFileSync(templatePath).toString(); replacements.forEach(([placeholder, value]) => { + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped const placeholderRegex = new RegExp(escapeStringForRegex(placeholder), 'g'); templateCode = templateCode.replace(placeholderRegex, value); }); diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 3d2fd7c80bb4..dab8c767c54f 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -115,6 +115,7 @@ export default function wrappingLoader( // Add a slash at the beginning .replace(/(.*)/, '/$1') // Pull off the file extension + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input .replace(new RegExp(`\\.(${pageExtensionRegex})`), '') // Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into // just `/xyz` diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 0abe309c7406..75f27bb9e649 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -462,7 +462,7 @@ function isMatchingRule(rule: WebpackModuleRule, projectDir: string): boolean { // // The next 11 option is ugly, but thankfully 'next', 'babel', and 'loader' do appear in it in the same order as in // 'next-babel-loader', so we can use the same regex to test for both. - if (!useEntries.some(entry => entry?.loader && new RegExp('next.*(babel|swc).*loader').test(entry.loader))) { + if (!useEntries.some(entry => entry?.loader && /next.*(babel|swc).*loader/.test(entry.loader))) { return false; } diff --git a/packages/nextjs/src/edge/index.ts b/packages/nextjs/src/edge/index.ts index 5c4d39c3bdc7..310d308837d2 100644 --- a/packages/nextjs/src/edge/index.ts +++ b/packages/nextjs/src/edge/index.ts @@ -48,6 +48,7 @@ export function init(options: VercelEdgeOptions = {}): void { // Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with // the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax. + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`); const defaultRewriteFramesIntegration = new RewriteFrames({ diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index f380f949ef6b..6aa9d1a0cb0e 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -131,6 +131,7 @@ function addServerIntegrations(options: NodeOptions): void { // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so // we can read in the project directory from the currently running process const distDirAbsPath = path.resolve(distDirName).replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath)); const defaultRewriteFramesIntegration = new RewriteFrames({ diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index 96aa53e1f9f4..de4444f28992 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -51,6 +51,7 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame { let strippedFilename; if (svelteKitBuildOutDir) { strippedFilename = filename.replace( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`), '', ); diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index a418dd0dcaa0..eea862e9d901 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -139,6 +139,7 @@ export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePlugi transform: async (code, id) => { let modifiedCode = code; + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input + escaped anyway const isServerHooksFile = new RegExp(`/${escapeStringForRegex(serverHooksFile)}(.(js|ts|mjs|mts))?`).test(id); if (isServerHooksFile) { @@ -195,6 +196,7 @@ export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePlugi if (fs.existsSync(mapFile)) { const mapContent = (await fs.promises.readFile(mapFile, 'utf-8')).toString(); const cleanedMapContent = mapContent.replace( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- no user input + escaped anyway new RegExp(escapeStringForRegex(WRAPPED_MODULE_SUFFIX), 'gm'), '', ); diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index ab6451f1ec1c..d1f609bd658d 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -412,6 +412,7 @@ export const extractOriginalRoute = ( const orderedKeys = keys.sort((a, b) => a.offset - b.offset); // add d flag for getting indices from regexp result + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- regexp comes from express.js const pathRegex = new RegExp(regexp, `${regexp.flags}d`); /** * use custom type cause of TS error with missing indices in RegExpExecArray diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts index 6ad839f2b920..438af21ac744 100644 --- a/packages/utils/src/tracing.ts +++ b/packages/utils/src/tracing.ts @@ -3,6 +3,7 @@ import type { DynamicSamplingContext, PropagationContext, TraceparentData } from import { baggageHeaderToDynamicSamplingContext } from './baggage'; import { uuid4 } from './misc'; +// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp is used for readability here export const TRACEPARENT_REGEXP = new RegExp( '^[ \\t]*' + // whitespace '([0-9a-f]{32})?' + // trace_id