Skip to content

Commit

Permalink
Review Error-throwing sites for whether they can sabotage SSI (#4627)
Browse files Browse the repository at this point in the history
  • Loading branch information
szegedi authored Aug 30, 2024
1 parent 735ac0d commit 34a651e
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
5 changes: 5 additions & 0 deletions packages/dd-trace/src/profiling/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}.`)
}
}
Expand Down
12 changes: 10 additions & 2 deletions packages/dd-trace/src/profiling/ssi-heuristics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/dd-trace/src/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 34a651e

Please sign in to comment.