diff --git a/docs/probes.md b/docs/probes.md index 7bb01e85646..5bdbaf61ad4 100644 --- a/docs/probes.md +++ b/docs/probes.md @@ -194,7 +194,7 @@ If the probe finds no binary files, it returns a single OutcomeFalse. **Implementation**: The probe analyzes the repository's workflows for known dangerous patterns. -**Outcomes**: The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. +**Outcomes**: The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. Each finding may include a suggested patch to fix the respective script injection. If no dangerous patterns are found, the probe returns one finding with OutcomeFalse. diff --git a/probes/hasDangerousWorkflowScriptInjection/def.yml b/probes/hasDangerousWorkflowScriptInjection/def.yml index 692cedb42e7..3b0b0351702 100644 --- a/probes/hasDangerousWorkflowScriptInjection/def.yml +++ b/probes/hasDangerousWorkflowScriptInjection/def.yml @@ -20,7 +20,7 @@ motivation: > implementation: > The probe analyzes the repository's workflows for known dangerous patterns. outcome: - - The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. Each finding includes a suggested patch to fix the respective script injection. + - The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. Each finding may include a suggested patch to fix the respective script injection. - If no dangerous patterns are found, the probe returns one finding with OutcomeFalse. remediation: onOutcome: True diff --git a/probes/hasDangerousWorkflowScriptInjection/impl.go b/probes/hasDangerousWorkflowScriptInjection/impl.go index 69efee10118..5c82de58b30 100644 --- a/probes/hasDangerousWorkflowScriptInjection/impl.go +++ b/probes/hasDangerousWorkflowScriptInjection/impl.go @@ -58,7 +58,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } var findings []finding.Finding - var curr string + var currWorkflow string var workflow *actionlint.Workflow var content []byte var errs []*actionlint.Error @@ -83,8 +83,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { findings = append(findings, *f) wp := path.Join(localPath, e.File.Path) - if curr != wp { - curr = wp + if currWorkflow != wp { + // update current open file if injection in different file + currWorkflow = wp content, err = os.ReadFile(wp) if err != nil { continue diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go index 4c10fbb1de5..5f464039616 100644 --- a/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go @@ -61,10 +61,11 @@ func patchWorkflow(f checker.File, content []byte, workflow *actionlint.Workflow unsafeVar := strings.TrimSpace(f.Snippet) lines := bytes.Split(content, []byte("\n")) - if f.Offset == 0 || int(f.Offset) >= len(lines) { + runCmdIndex := int(f.Offset - 1) + + if runCmdIndex < 0 || runCmdIndex >= len(lines) { return []byte(""), sce.WithMessage(sce.ErrScorecardInternal, "Invalid dangerous workflow offset") } - runCmdIndex := f.Offset - 1 unsafePattern, err := getUnsafePattern(unsafeVar) if err != nil { @@ -209,10 +210,10 @@ func useExistingEnvvars( } // Replaces all instances of the given script injection variable with the safe environment variable. -func replaceUnsafeVarWithEnvvar(lines [][]byte, pattern unsafePattern, runIndex uint) { +func replaceUnsafeVarWithEnvvar(lines [][]byte, pattern unsafePattern, runIndex int) { runIndent := getIndent(lines[runIndex]) for i, line := range lines[runIndex:] { - currLine := int(runIndex) + i + currLine := runIndex + i if i > 0 && isParentLevelIndent(lines[currLine], runIndent) { // anything at the same indent as the first line of the `- run:` block will mean the end of the run block. break