From 34a651effd9202d6ee1cffd2930886f615fe7956 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Fri, 30 Aug 2024 16:43:25 +0200 Subject: [PATCH] Review Error-throwing sites for whether they can sabotage SSI (#4627) --- packages/dd-trace/src/profiling/config.js | 5 +++++ packages/dd-trace/src/profiling/ssi-heuristics.js | 12 ++++++++++-- packages/dd-trace/src/proxy.js | 1 + 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/profiling/config.js b/packages/dd-trace/src/profiling/config.js index 8d399c68bf5..538400aaa7a 100644 --- a/packages/dd-trace/src/profiling/config.js +++ b/packages/dd-trace/src/profiling/config.js @@ -97,6 +97,11 @@ class Config { const samplingContextsAvailable = process.platform !== 'win32' function checkOptionAllowed (option, description, condition) { if (option && !condition) { + // injection hardening: all of these can only happen if user explicitly + // sets an environment variable to its non-default value on the platform. + // In practical terms, it'd require someone explicitly turning on OOM + // monitoring, code hotspots, endpoint profiling, or CPU profiling on + // Windows, where it is not supported. throw new Error(`${description} not supported on ${process.platform}.`) } } diff --git a/packages/dd-trace/src/profiling/ssi-heuristics.js b/packages/dd-trace/src/profiling/ssi-heuristics.js index 9910b9273bc..4790ae2b9b5 100644 --- a/packages/dd-trace/src/profiling/ssi-heuristics.js +++ b/packages/dd-trace/src/profiling/ssi-heuristics.js @@ -3,6 +3,7 @@ const telemetryMetrics = require('../telemetry/metrics') const profilersNamespace = telemetryMetrics.manager.namespace('profilers') const dc = require('dc-polyfill') +const log = require('../log') // If the process lives for at least 30 seconds, it's considered long-lived const DEFAULT_LONG_LIVED_THRESHOLD = 30000 @@ -40,9 +41,14 @@ class SSIHeuristics { const longLivedThreshold = config.profiling.longLivedThreshold || DEFAULT_LONG_LIVED_THRESHOLD if (typeof longLivedThreshold !== 'number' || longLivedThreshold <= 0) { - throw new Error('Long-lived threshold must be a positive number') + this.longLivedThreshold = DEFAULT_LONG_LIVED_THRESHOLD + log.warn( + `Invalid SSIHeuristics.longLivedThreshold value: ${config.profiling.longLivedThreshold}. ` + + `Using default value: ${DEFAULT_LONG_LIVED_THRESHOLD}` + ) + } else { + this.longLivedThreshold = longLivedThreshold } - this.longLivedThreshold = longLivedThreshold this.hasSentProfiles = false this.noSpan = true @@ -94,6 +100,8 @@ class SSIHeuristics { }) break default: + // injection hardening: only usage is internal, one call site with + // a function and another with undefined, so we can throw here. throw new TypeError('callback must be a function or undefined') } } diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index c3b865226ed..dd30fa89c88 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -202,6 +202,7 @@ class Tracer extends NoopProxy { profilerStarted () { if (!this._profilerStarted) { + // injection hardening: this is only ever invoked from tests. throw new Error('profilerStarted() must be called after init()') } return this._profilerStarted