|
| 1 | +package template |
| 2 | + |
| 3 | +import ( |
| 4 | + "bytes" |
| 5 | + "strings" |
| 6 | + "testing" |
| 7 | + |
| 8 | + "github.com/sourcegraph/sourcegraph/lib/batches/execution" |
| 9 | + "github.com/sourcegraph/sourcegraph/lib/batches/git" |
| 10 | +) |
| 11 | + |
| 12 | +// TestVULN91_NoShellInjectionFromFilenames is a regression test for VULN-91 |
| 13 | +// (HackerOne report 3767160). It verifies that attacker-controlled filenames |
| 14 | +// surfaced through the various filename-bearing template variables cannot |
| 15 | +// inject shell metacharacters into the rendered step script. |
| 16 | +// |
| 17 | +// The PoC in the report relies on filenames like |
| 18 | +// |
| 19 | +// `id > /tmp/PWNED`.go |
| 20 | +// foo.go; echo INJECTED; #.go |
| 21 | +// |
| 22 | +// being rendered verbatim through `${{ join repository.search_result_paths " " }}` |
| 23 | +// and friends, then executed under /bin/sh. After the fix, each filename must |
| 24 | +// be shell-quoted so the metacharacters lose their shell meaning. |
| 25 | +func TestVULN91_NoShellInjectionFromFilenames(t *testing.T) { |
| 26 | + maliciousFilenames := []string{ |
| 27 | + "main.go", |
| 28 | + "`id > /tmp/PWNED && cat /tmp/PWNED`.go", |
| 29 | + "foo.go; echo INJECTED_$(whoami) > /tmp/PWNED2; #.go", |
| 30 | + "with space.go", |
| 31 | + "with'quote.go", |
| 32 | + "with\nnewline.go", |
| 33 | + } |
| 34 | + |
| 35 | + // These template snippets all unconditionally splat filename-bearing |
| 36 | + // variables into a shell command. None of them may contain raw shell |
| 37 | + // metacharacters after rendering. |
| 38 | + templates := map[string]string{ |
| 39 | + "step run via repository.search_result_paths": `gofmt -w ${{ join repository.search_result_paths " " }}`, |
| 40 | + "step run via previous_step.modified_files": `gofmt -w ${{ join previous_step.modified_files " " }}`, |
| 41 | + "step run via previous_step.added_files": `gofmt -w ${{ join previous_step.added_files " " }}`, |
| 42 | + "step run via previous_step.deleted_files": `rm -f ${{ join previous_step.deleted_files " " }}`, |
| 43 | + "step run via previous_step.renamed_files": `touch ${{ join previous_step.renamed_files " " }}`, |
| 44 | + "step run via steps.modified_files": `gofmt -w ${{ join steps.modified_files " " }}`, |
| 45 | + "step run via direct repository fmt": `gofmt -w ${{ repository.search_result_paths }}`, |
| 46 | + } |
| 47 | + |
| 48 | + stepCtx := &StepContext{ |
| 49 | + Repository: Repository{ |
| 50 | + Name: "github.com/example/repo", |
| 51 | + Branch: "main", |
| 52 | + FileMatches: maliciousFilenames, |
| 53 | + }, |
| 54 | + PreviousStep: execution.AfterStepResult{ |
| 55 | + ChangedFiles: git.Changes{ |
| 56 | + Modified: maliciousFilenames, |
| 57 | + Added: maliciousFilenames, |
| 58 | + Deleted: maliciousFilenames, |
| 59 | + Renamed: maliciousFilenames, |
| 60 | + }, |
| 61 | + }, |
| 62 | + Steps: StepsContext{ |
| 63 | + Changes: git.Changes{ |
| 64 | + Modified: maliciousFilenames, |
| 65 | + Added: maliciousFilenames, |
| 66 | + Deleted: maliciousFilenames, |
| 67 | + Renamed: maliciousFilenames, |
| 68 | + }, |
| 69 | + }, |
| 70 | + } |
| 71 | + |
| 72 | + // Tokens that, if they appear unescaped in the rendered output, would |
| 73 | + // trigger shell command substitution, command chaining, or redirection. |
| 74 | + dangerousSubstrings := []string{ |
| 75 | + "`id", |
| 76 | + "$(", |
| 77 | + "; echo", |
| 78 | + "> /tmp/PWNED", |
| 79 | + "PWNED2", |
| 80 | + } |
| 81 | + |
| 82 | + for name, tmpl := range templates { |
| 83 | + t.Run(name, func(t *testing.T) { |
| 84 | + var out bytes.Buffer |
| 85 | + if err := RenderStepTemplate("vuln-91", tmpl, &out, stepCtx); err != nil { |
| 86 | + t.Fatalf("RenderStepTemplate returned error: %v", err) |
| 87 | + } |
| 88 | + rendered := out.String() |
| 89 | + for _, bad := range dangerousSubstrings { |
| 90 | + if containsUnquoted(rendered, bad) { |
| 91 | + t.Errorf("rendered output contains unescaped shell metasequence %q\nrendered: %s", bad, rendered) |
| 92 | + } |
| 93 | + } |
| 94 | + }) |
| 95 | + } |
| 96 | + |
| 97 | + // Also exercise the ChangesetTemplateContext path. |
| 98 | + tmplCtx := &ChangesetTemplateContext{ |
| 99 | + Repository: Repository{ |
| 100 | + Name: "github.com/example/repo", |
| 101 | + Branch: "main", |
| 102 | + FileMatches: maliciousFilenames, |
| 103 | + }, |
| 104 | + Steps: StepsContext{ |
| 105 | + Changes: git.Changes{ |
| 106 | + Modified: maliciousFilenames, |
| 107 | + Added: maliciousFilenames, |
| 108 | + Deleted: maliciousFilenames, |
| 109 | + Renamed: maliciousFilenames, |
| 110 | + }, |
| 111 | + }, |
| 112 | + } |
| 113 | + ctTemplates := map[string]string{ |
| 114 | + "changeset via repository.search_result_paths": `body: ${{ join repository.search_result_paths " " }}`, |
| 115 | + "changeset via steps.modified_files": `body: ${{ join steps.modified_files " " }}`, |
| 116 | + } |
| 117 | + for name, tmpl := range ctTemplates { |
| 118 | + t.Run(name, func(t *testing.T) { |
| 119 | + rendered, err := RenderChangesetTemplateField("vuln-91", tmpl, tmplCtx) |
| 120 | + if err != nil { |
| 121 | + t.Fatalf("RenderChangesetTemplateField returned error: %v", err) |
| 122 | + } |
| 123 | + for _, bad := range dangerousSubstrings { |
| 124 | + if containsUnquoted(rendered, bad) { |
| 125 | + t.Errorf("rendered output contains unescaped shell metasequence %q\nrendered: %s", bad, rendered) |
| 126 | + } |
| 127 | + } |
| 128 | + }) |
| 129 | + } |
| 130 | +} |
| 131 | + |
| 132 | +// containsUnquoted reports whether needle occurs in haystack outside of a |
| 133 | +// single-quoted shell context. After shellquote.Join, dangerous bytes are |
| 134 | +// either backslash-escaped (e.g. \`) or wrapped in single quotes that |
| 135 | +// terminate the surrounding string literal, so any naive substring match |
| 136 | +// reliably finds an injection. We additionally allow the substring to appear |
| 137 | +// inside a single-quoted region (which shell does NOT interpret) to avoid |
| 138 | +// false positives on the literal text of the (now safely quoted) filename. |
| 139 | +func containsUnquoted(haystack, needle string) bool { |
| 140 | + for i := 0; i+len(needle) <= len(haystack); i++ { |
| 141 | + if haystack[i:i+len(needle)] != needle { |
| 142 | + continue |
| 143 | + } |
| 144 | + // Count single quotes before this position; if odd, we're inside a |
| 145 | + // single-quoted region and the bytes are harmless. |
| 146 | + quotes := strings.Count(haystack[:i], "'") |
| 147 | + if quotes%2 == 0 { |
| 148 | + return true |
| 149 | + } |
| 150 | + } |
| 151 | + return false |
| 152 | +} |
0 commit comments