|
| 1 | +/* |
| 2 | + * MIGRATION EXAMPLE: TemplateInjectionRecipe using YamlScalarAccessor trait |
| 3 | + * |
| 4 | + * This file demonstrates what TemplateInjectionRecipe would look like after |
| 5 | + * migrating from YamlHelper utility class to YamlScalarAccessor trait. |
| 6 | + * |
| 7 | + * Key changes: |
| 8 | + * 1. Add "implements YamlScalarAccessor" to visitor class |
| 9 | + * 2. Replace "YamlHelper.getScalarValue(x)" with "getScalarValue(x)" |
| 10 | + * 3. No import changes needed (trait is an interface) |
| 11 | + * |
| 12 | + * Lines changed: 4 (lines 125, 146, 164 in original) |
| 13 | + * Import changes: 0 |
| 14 | + * Behavior changes: 0 (identical functionality) |
| 15 | + */ |
| 16 | + |
| 17 | +package org.openrewrite.github.security; |
| 18 | + |
| 19 | +import lombok.EqualsAndHashCode; |
| 20 | +import lombok.Value; |
| 21 | +import org.jspecify.annotations.Nullable; |
| 22 | +import org.openrewrite.*; |
| 23 | +import org.openrewrite.marker.SearchResult; |
| 24 | +import org.openrewrite.yaml.JsonPathMatcher; |
| 25 | +import org.openrewrite.yaml.YamlIsoVisitor; |
| 26 | +import org.openrewrite.yaml.tree.Yaml; |
| 27 | +import org.openrewrite.github.traits.YamlScalarAccessor; // NEW IMPORT |
| 28 | + |
| 29 | +import java.util.Arrays; |
| 30 | +import java.util.HashSet; |
| 31 | +import java.util.Set; |
| 32 | +import java.util.regex.Matcher; |
| 33 | +import java.util.regex.Pattern; |
| 34 | + |
| 35 | +@Value |
| 36 | +@EqualsAndHashCode(callSuper = false) |
| 37 | +public class TemplateInjectionRecipe extends Recipe { |
| 38 | + |
| 39 | + // User-controllable contexts that can lead to injection vulnerabilities |
| 40 | + private static final Set<String> DANGEROUS_CONTEXTS = new HashSet<>(Arrays.asList( |
| 41 | + "github.event.pull_request.title", |
| 42 | + "github.event.pull_request.body", |
| 43 | + "github.event.pull_request.head.ref", |
| 44 | + "github.event.pull_request.head.label", |
| 45 | + "github.event.pull_request.head.repo.default_branch", |
| 46 | + "github.event.pull_request.base.ref", |
| 47 | + "github.event.issue.title", |
| 48 | + "github.event.issue.body", |
| 49 | + "github.event.comment.body", |
| 50 | + "github.event.review.body", |
| 51 | + "github.event.pages[0].page_name", |
| 52 | + "github.event.commits[0].message", |
| 53 | + "github.event.head_commit.message", |
| 54 | + "github.event.commits[0].author.name", |
| 55 | + "github.event.commits[0].author.email", |
| 56 | + "github.head_ref" |
| 57 | + )); |
| 58 | + |
| 59 | + // Contexts that reference user-controllable step outputs |
| 60 | + private static final Pattern STEPS_OUTPUT_PATTERN = Pattern.compile( |
| 61 | + "steps\\.[^.]+\\.outputs\\.[^\\s}]+" |
| 62 | + ); |
| 63 | + |
| 64 | + // Actions known to have code injection sinks |
| 65 | + private static final Set<String> CODE_INJECTION_ACTIONS = new HashSet<>(Arrays.asList( |
| 66 | + "actions/github-script", |
| 67 | + "amadevus/pwsh-script", |
| 68 | + "jannekem/run-python-script-action", |
| 69 | + "cardinalby/js-eval-action" |
| 70 | + )); |
| 71 | + |
| 72 | + // Expression pattern to find GitHub Actions expressions |
| 73 | + private static final Pattern EXPRESSION_PATTERN = Pattern.compile( |
| 74 | + "\\$\\{\\{([^}]+)\\}\\}" |
| 75 | + ); |
| 76 | + |
| 77 | + @Override |
| 78 | + public String getDisplayName() { |
| 79 | + return "Find template injection vulnerabilities"; |
| 80 | + } |
| 81 | + |
| 82 | + @Override |
| 83 | + public String getDescription() { |
| 84 | + return "Find GitHub Actions workflows vulnerable to template injection attacks. These occur when user-controllable " + |
| 85 | + "input (like pull request titles, issue bodies, or commit messages) is used directly in `run` commands or " + |
| 86 | + "`script` inputs without proper escaping. Attackers can exploit this to execute arbitrary code. " + |
| 87 | + "Based on [zizmor's `template-injection` audit](https://github.com/woodruffw/zizmor/blob/main/crates/zizmor/src/audit/template_injection.rs)."; |
| 88 | + } |
| 89 | + |
| 90 | + @Override |
| 91 | + public TreeVisitor<?, ExecutionContext> getVisitor() { |
| 92 | + return Preconditions.check( |
| 93 | + new FindSourceFiles(".github/workflows/*.yml"), |
| 94 | + new TemplateInjectionVisitor() |
| 95 | + ); |
| 96 | + } |
| 97 | + |
| 98 | + // CHANGE: Added "implements YamlScalarAccessor" |
| 99 | + private static class TemplateInjectionVisitor extends YamlIsoVisitor<ExecutionContext> |
| 100 | + implements YamlScalarAccessor { |
| 101 | + |
| 102 | + private static final JsonPathMatcher STEP_RUN_MATCHER = new JsonPathMatcher("$..steps[*].run"); |
| 103 | + private static final JsonPathMatcher STEP_USES_MATCHER = new JsonPathMatcher("$..steps[*].uses"); |
| 104 | + private static final JsonPathMatcher STEP_SCRIPT_MATCHER = new JsonPathMatcher("$..steps[*].with.script"); |
| 105 | + |
| 106 | + @Override |
| 107 | + public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { |
| 108 | + Yaml.Mapping.Entry mappingEntry = super.visitMappingEntry(entry, ctx); |
| 109 | + |
| 110 | + // Check run commands for injection vulnerabilities |
| 111 | + if (STEP_RUN_MATCHER.matches(getCursor())) { |
| 112 | + return checkRunEntry(mappingEntry); |
| 113 | + } |
| 114 | + |
| 115 | + // Check uses entries for code injection actions |
| 116 | + if (STEP_USES_MATCHER.matches(getCursor())) { |
| 117 | + return checkUsesEntry(mappingEntry); |
| 118 | + } |
| 119 | + |
| 120 | + // Check script inputs for code injection actions |
| 121 | + if (STEP_SCRIPT_MATCHER.matches(getCursor())) { |
| 122 | + return checkScriptEntry(mappingEntry); |
| 123 | + } |
| 124 | + |
| 125 | + return mappingEntry; |
| 126 | + } |
| 127 | + |
| 128 | + private Yaml.Mapping.Entry checkRunEntry(Yaml.Mapping.Entry entry) { |
| 129 | + // CHANGE: Removed "YamlHelper." prefix - now using trait method |
| 130 | + String runCommand = getScalarValue(entry.getValue()); |
| 131 | + if (runCommand == null) { |
| 132 | + return entry; |
| 133 | + } |
| 134 | + |
| 135 | + // Check for dangerous template expressions in run commands |
| 136 | + String vulnerableContext = findVulnerableContext(runCommand); |
| 137 | + if (vulnerableContext != null) { |
| 138 | + String message; |
| 139 | + if (vulnerableContext.startsWith("User-controlled input")) { |
| 140 | + message = "Potential template injection vulnerability. " + vulnerableContext + " used in run command without proper escaping."; |
| 141 | + } else { |
| 142 | + message = "Potential template injection vulnerability. User-controlled input '" + vulnerableContext + "' used in run command without proper escaping."; |
| 143 | + } |
| 144 | + return SearchResult.found(entry, message); |
| 145 | + } |
| 146 | + |
| 147 | + return entry; |
| 148 | + } |
| 149 | + |
| 150 | + private Yaml.Mapping.Entry checkUsesEntry(Yaml.Mapping.Entry entry) { |
| 151 | + // CHANGE: Removed "YamlHelper." prefix - now using trait method |
| 152 | + String usesValue = getScalarValue(entry.getValue()); |
| 153 | + if (usesValue == null) { |
| 154 | + return entry; |
| 155 | + } |
| 156 | + |
| 157 | + // Check if this is a code injection action |
| 158 | + for (String dangerousAction : CODE_INJECTION_ACTIONS) { |
| 159 | + if (usesValue.startsWith(dangerousAction)) { |
| 160 | + // Flag the use of a code injection action as potentially dangerous |
| 161 | + return SearchResult.found(entry, |
| 162 | + "Potential code injection in script input. User-controlled content in script execution context."); |
| 163 | + } |
| 164 | + } |
| 165 | + |
| 166 | + return entry; |
| 167 | + } |
| 168 | + |
| 169 | + private Yaml.Mapping.Entry checkScriptEntry(Yaml.Mapping.Entry entry) { |
| 170 | + // CHANGE: Removed "YamlHelper." prefix - now using trait method |
| 171 | + String scriptContent = getScalarValue(entry.getValue()); |
| 172 | + if (scriptContent == null) { |
| 173 | + return entry; |
| 174 | + } |
| 175 | + |
| 176 | + // Check for vulnerable contexts in the script content |
| 177 | + String vulnerableContext = findVulnerableContext(scriptContent); |
| 178 | + if (vulnerableContext != null) { |
| 179 | + String message; |
| 180 | + if (vulnerableContext.startsWith("User-controlled input")) { |
| 181 | + message = "Potential code injection in script. " + vulnerableContext + " used in script without proper escaping."; |
| 182 | + } else { |
| 183 | + message = "Potential code injection in script. User-controlled input '" + vulnerableContext + |
| 184 | + "' used in script without proper escaping."; |
| 185 | + } |
| 186 | + return SearchResult.found(entry, message); |
| 187 | + } |
| 188 | + |
| 189 | + return entry; |
| 190 | + } |
| 191 | + |
| 192 | + private @Nullable String findVulnerableContext(String content) { |
| 193 | + // Find all expressions in the content |
| 194 | + Matcher matcher = EXPRESSION_PATTERN.matcher(content); |
| 195 | + |
| 196 | + while (matcher.find()) { |
| 197 | + String expression = matcher.group(1).trim(); |
| 198 | + |
| 199 | + // Check for complex expressions containing dangerous contexts first |
| 200 | + if (isComplexExpression(expression) && containsDangerousContextInExpression(expression)) { |
| 201 | + return "User-controlled input in complex expression"; |
| 202 | + } |
| 203 | + |
| 204 | + // Check for directly dangerous contexts |
| 205 | + for (String dangerousContext : DANGEROUS_CONTEXTS) { |
| 206 | + if (expression.contains(dangerousContext)) { |
| 207 | + return dangerousContext; |
| 208 | + } |
| 209 | + } |
| 210 | + |
| 211 | + // Check for steps outputs (which may contain user input) |
| 212 | + Matcher stepsMatcher = STEPS_OUTPUT_PATTERN.matcher(expression); |
| 213 | + if (stepsMatcher.find()) { |
| 214 | + return stepsMatcher.group(); |
| 215 | + } |
| 216 | + } |
| 217 | + |
| 218 | + return null; |
| 219 | + } |
| 220 | + |
| 221 | + private boolean isComplexExpression(String expression) { |
| 222 | + // Consider it complex if it contains function calls or multiple contexts |
| 223 | + return expression.contains("(") && expression.contains(")"); |
| 224 | + } |
| 225 | + |
| 226 | + private boolean containsDangerousContextInExpression(String expression) { |
| 227 | + // Check for function calls or complex expressions that might contain dangerous contexts |
| 228 | + for (String dangerousContext : DANGEROUS_CONTEXTS) { |
| 229 | + if (expression.contains(dangerousContext)) { |
| 230 | + return true; |
| 231 | + } |
| 232 | + } |
| 233 | + |
| 234 | + return false; |
| 235 | + } |
| 236 | + |
| 237 | + } |
| 238 | +} |
0 commit comments