Skip to content

Commit 309018f

Browse files
authored
fix: segfault in template caching logic (#6421)
* fix: segfault in template caching logic when templates had no executable requests after option updates. the cached templates could end up with 0 requests and no flow execution path, resulting in a nil engine pointer that was later derefer w/o validation. bug seq: caching template (w/ valid requests) -> get cached template -> `*ExecutorOptions.Options` copied and modified (inconsistent) -> requests updated (with new options -- some may be invalid, and without recompile) -> template returned w/o validation -> `compileProtocolRequests` -> `NewTemplateExecuter` receive empty requests + empty flow = nil engine -> `*TemplateExecuter.{Compile,Execute}` invoked on nil engine = panic. RCA: 1. `*ExecutorOptions.ApplyNewEngineOptions` overwriting many fields. 2. copy op pointless; create a copy of options and then immediately replace it with original pointer. 3. missing executable requests validation after cached templates is reconstructed with updated options. Thus, this affected `--automatic-scan` mode where tech detection templates often have conditional requests that may be filtered based on runtime options. Fixes #6417 Signed-off-by: Dwi Siswanto <git@dw1.io> * fix(templates): recompile workflow with `tplCopy.Options` Signed-off-by: Dwi Siswanto <git@dw1.io> * fix(templates): strengthen cache hit guard Signed-off-by: Dwi Siswanto <git@dw1.io> * fix(protocols): skips template-specific fields Signed-off-by: Dwi Siswanto <git@dw1.io> --------- Signed-off-by: Dwi Siswanto <git@dw1.io>
1 parent 5e9ada2 commit 309018f

File tree

2 files changed

+59
-23
lines changed

2 files changed

+59
-23
lines changed

pkg/protocols/protocols.go

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -447,21 +447,8 @@ func (e *ExecutorOptions) ApplyNewEngineOptions(n *ExecutorOptions) {
447447
return
448448
}
449449

450-
// The types.Options include the ExecutionID among other things
451450
e.Options = n.Options.Copy()
452-
453-
// Keep the template-specific fields, but replace the rest
454-
/*
455-
e.TemplateID = n.TemplateID
456-
e.TemplatePath = n.TemplatePath
457-
e.TemplateInfo = n.TemplateInfo
458-
e.TemplateVerifier = n.TemplateVerifier
459-
e.RawTemplate = n.RawTemplate
460-
e.Variables = n.Variables
461-
e.Constants = n.Constants
462-
*/
463451
e.Output = n.Output
464-
e.Options = n.Options
465452
e.IssuesClient = n.IssuesClient
466453
e.Progress = n.Progress
467454
e.RateLimiter = n.RateLimiter
@@ -470,19 +457,13 @@ func (e *ExecutorOptions) ApplyNewEngineOptions(n *ExecutorOptions) {
470457
e.Browser = n.Browser
471458
e.Interactsh = n.Interactsh
472459
e.HostErrorsCache = n.HostErrorsCache
473-
e.StopAtFirstMatch = n.StopAtFirstMatch
474-
e.ExcludeMatchers = n.ExcludeMatchers
475460
e.InputHelper = n.InputHelper
476461
e.FuzzParamsFrequency = n.FuzzParamsFrequency
477462
e.FuzzStatsDB = n.FuzzStatsDB
478463
e.DoNotCache = n.DoNotCache
479464
e.Colorizer = n.Colorizer
480465
e.WorkflowLoader = n.WorkflowLoader
481466
e.ResumeCfg = n.ResumeCfg
482-
e.ProtocolType = n.ProtocolType
483-
e.Flow = n.Flow
484-
e.IsMultiProtocol = n.IsMultiProtocol
485-
e.templateCtxStore = n.templateCtxStore
486467
e.JsCompiler = n.JsCompiler
487468
e.AuthProvider = n.AuthProvider
488469
e.TemporaryDirectory = n.TemporaryDirectory

pkg/templates/compile.go

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ func Parse(filePath string, preprocessor Preprocessor, options *protocols.Execut
6464
newBase.TemplateInfo = tplCopy.Options.TemplateInfo
6565
newBase.TemplateVerifier = tplCopy.Options.TemplateVerifier
6666
newBase.RawTemplate = tplCopy.Options.RawTemplate
67+
68+
if tplCopy.Options.Variables.Len() > 0 {
69+
newBase.Variables = tplCopy.Options.Variables
70+
}
71+
if len(tplCopy.Options.Constants) > 0 {
72+
newBase.Constants = tplCopy.Options.Constants
73+
}
6774
tplCopy.Options = newBase
6875

6976
tplCopy.Options.ApplyNewEngineOptions(options)
@@ -156,12 +163,16 @@ func Parse(filePath string, preprocessor Preprocessor, options *protocols.Execut
156163
// Compile the workflow request
157164
if len(template.Workflows) > 0 {
158165
compiled := &template.Workflow
159-
compileWorkflow(filePath, preprocessor, options, compiled, options.WorkflowLoader)
166+
compileWorkflow(filePath, preprocessor, tplCopy.Options, compiled, tplCopy.Options.WorkflowLoader)
160167
template.CompiledWorkflow = compiled
161-
template.CompiledWorkflow.Options = options
168+
template.CompiledWorkflow.Options = tplCopy.Options
169+
}
170+
171+
if isCachedTemplateValid(template) {
172+
// options.Logger.Error().Msgf("returning cached template %s after recompiling %d requests", tplCopy.Options.TemplateID, tplCopy.Requests())
173+
return template, nil
162174
}
163-
// options.Logger.Error().Msgf("returning cached template %s after recompiling %d requests", tplCopy.Options.TemplateID, tplCopy.Requests())
164-
return template, nil
175+
// else: fallthrough to re-parse template from scratch
165176
}
166177
}
167178

@@ -579,6 +590,50 @@ func parseTemplate(data []byte, srcOptions *protocols.ExecutorOptions) (*Templat
579590
return template, nil
580591
}
581592

593+
// isCachedTemplateValid validates that a cached template is still usable after
594+
// option updates
595+
func isCachedTemplateValid(template *Template) bool {
596+
// no requests or workflows
597+
if template.Requests() == 0 && len(template.Workflows) == 0 {
598+
return false
599+
}
600+
601+
// options not initialized
602+
if template.Options == nil {
603+
return false
604+
}
605+
606+
// executer not available for non-workflow template
607+
if len(template.Workflows) == 0 && template.Executer == nil {
608+
return false
609+
}
610+
611+
// compiled workflow not available
612+
if len(template.Workflows) > 0 && template.CompiledWorkflow == nil {
613+
return false
614+
}
615+
616+
// template ID mismatch
617+
if template.Options.TemplateID != template.ID {
618+
return false
619+
}
620+
621+
// executer exists but no requests or flow available
622+
if template.Executer != nil {
623+
// NOTE(dwisiswant0): This is a basic sanity check since we can't access
624+
// private fields, but we can check requests tho
625+
if template.Requests() == 0 && template.Options.Flow == "" {
626+
return false
627+
}
628+
}
629+
630+
if template.Options.Options == nil {
631+
return false
632+
}
633+
634+
return true
635+
}
636+
582637
var (
583638
jsCompiler *compiler.Compiler
584639
jsCompilerOnce = sync.OnceFunc(func() {

0 commit comments

Comments
 (0)