CVE-2022-45381

@@ -27,9 +27,14 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.FilePath;
 import hudson.model.TaskListener;
+import jenkins.util.SystemProperties;
 import org.apache.commons.configuration2.AbstractConfiguration;
 import org.apache.commons.configuration2.Configuration;
 import org.apache.commons.configuration2.ConfigurationConverter;
+import org.apache.commons.configuration2.interpol.ConfigurationInterpolator;
+import org.apache.commons.configuration2.interpol.DefaultLookups;
+import org.apache.commons.configuration2.interpol.InterpolatorSpecification;
+import org.apache.commons.configuration2.interpol.Lookup;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.pipeline.utility.steps.AbstractFileOrTextStepExecution;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
@@ -49,6 +54,16 @@
 public class ReadPropertiesStepExecution extends AbstractFileOrTextStepExecution<Map<String, Object>> {
     private static final long serialVersionUID = 1L;
 
+    private static final Map<String, Lookup> SAFE_PREFIX_INTERPOLATOR_LOOKUPS = new HashMap<String, Lookup>() {{
+        put(DefaultLookups.BASE64_DECODER.getPrefix(), DefaultLookups.BASE64_DECODER.getLookup());
+        put(DefaultLookups.BASE64_ENCODER.getPrefix(), DefaultLookups.BASE64_ENCODER.getLookup());
+        put(DefaultLookups.DATE.getPrefix(), DefaultLookups.DATE.getLookup());
+        put(DefaultLookups.URL_DECODER.getPrefix(), DefaultLookups.URL_DECODER.getLookup());
+        put(DefaultLookups.URL_ENCODER.getPrefix(), DefaultLookups.URL_ENCODER.getLookup());
+    }};
+
+    static /* not final */ String CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS = SystemProperties.getString(ReadPropertiesStepExecution.class.getName() + ".CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS");
+
     private transient ReadPropertiesStep step;
 
     protected ReadPropertiesStepExecution(@NonNull ReadPropertiesStep step, @NonNull StepContext context) {
@@ -122,22 +137,31 @@ private void addAll(Map<Object, Object> src, Map<String, Object> dst) {
     private Properties interpolateProperties(Properties properties) throws Exception {
         if ( properties == null)
             return null;
-        Configuration interpolatedProp;
         PrintStream logger = getLogger();
         try {
+            ConfigurationInterpolator configurationInterpolator = ConfigurationInterpolator.fromSpecification(
+                    new InterpolatorSpecification.Builder()
+                            .withPrefixLookups(
+                                    CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS == null ?
+                                            SAFE_PREFIX_INTERPOLATOR_LOOKUPS :
+                                            parseLookups(CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS)
+                            )
+                            .create()
+            );
             // Convert the Properties to a Configuration object in order to apply the interpolation
             Configuration conf = ConfigurationConverter.getConfiguration(properties);
+            conf.setInterpolator(configurationInterpolator);
 
             // Apply interpolation
-            interpolatedProp = ((AbstractConfiguration)conf).interpolatedConfiguration();
+            Configuration interpolatedProp = ((AbstractConfiguration)conf).interpolatedConfiguration();
+
+            // Convert back to properties
+            return ConfigurationConverter.getProperties(interpolatedProp);
         } catch (Exception e) {
             logger.println("Got exception while interpolating the variables: " + e.getMessage());
             logger.println("Returning the original properties list!");
             return properties;
         }
-
-        // Convert back to properties
-        return ConfigurationConverter.getProperties(interpolatedProp);
     }
 
     /**
@@ -150,4 +174,28 @@ private PrintStream getLogger() throws Exception {
         assert listener != null;
         return listener.getLogger();
     }
+
+    /*
+     * Method was copied from https://github.com/apache/commons-configuration/blob/aff776e3d4d81f1f856304306353be3279aec11a/src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java#L673-L687
+     * licensed under https://github.com/apache/commons-configuration/blob/aff776e3d4d81f1f856304306353be3279aec11a/LICENSE.txt
+     * and slightly modified.
+     */
+    private static Map<String, Lookup> parseLookups(final String str) {
+        final Map<String, Lookup> lookupMap = new HashMap<>();
+        if (StringUtils.isBlank(str))
+            return lookupMap;
+
+        try {
+            for (final String lookupName : str.split("[\\s,]+")) {
+                if (!lookupName.isEmpty()) {
+                    DefaultLookups lookup = DefaultLookups.valueOf(lookupName.toUpperCase());
+                    lookupMap.put(lookup.getPrefix(), lookup.getLookup());
+                }
+            }
+        } catch (IllegalArgumentException exc) {
+            throw new IllegalArgumentException("Invalid default lookups definition: " + str, exc);
+        }
+
+        return lookupMap;
+    }
 }

@@ -47,8 +47,17 @@
     </li>
     <li>
         <code>interpolate</code>:
-        Flag to indicate if the properties should be interpolated or not.
-        In case of error or cycling dependencies, the original properties will be returned.
+        Flag to indicate if the properties should be interpolated or not. <br>
+
+        Prefix interpolations allowed by default are: <code>urlDecoder</code>, <code>urlEncoder</code>,
+        <code>date</code>, <code>base64Decoder</code>, <code>base64Encoder</code>.
+
+        Default prefix interpolations can be overridden by setting the
+        <a href="https://www.jenkins.io/redirect/setting-system-properties">system property</a>: <br>
+        <code>org.jenkinsci.plugins.pipeline.utility.steps.conf.ReadPropertiesStepExecution.CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS</code><br>
+        <b>Note that overriding default prefix interpolations can be insecure depending on which ones you enable.</b>
+
+        In case of error or cyclic dependencies, the original properties will be returned.
     </li>
 </ul>
 <p>
@@ -73,4 +82,4 @@
         assert props.fullUrl = 'http://localhost/README.txt'
         </pre>
     </code>
-</p>
\ No newline at end of file
+</p>

@@ -28,6 +28,9 @@
 import hudson.model.Result;
 
 import static org.jenkinsci.plugins.pipeline.utility.steps.FilenameTestsUtils.separatorsToSystemEscaped;
+import static org.jenkinsci.plugins.pipeline.utility.steps.conf.ReadPropertiesStepExecution.CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import org.jenkinsci.plugins.pipeline.utility.steps.Messages;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
@@ -37,6 +40,8 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.FlagRule;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import java.io.File;
@@ -54,6 +59,8 @@ public class ReadPropertiesStepTest {
     public JenkinsRule j = new JenkinsRule();
     @Rule
     public TemporaryFolder temp = new TemporaryFolder();
+    @Rule
+    public FlagRule<String> customLookups = new FlagRule<>(() -> CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS, x -> CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS = x);
 
     @Before
     public void setup() throws Exception {
@@ -341,4 +348,75 @@ public void readFileAndTextInterpolatedWithCyclicValues() throws Exception {
                         "}", true));
         j.assertBuildStatusSuccess(p.scheduleBuild2(0));
     }
-}
\ No newline at end of file
+
+    @Issue("SECURITY-2949")
+    @Test
+    public void unsafeInterpolatorsDoNotInterpolate() throws Exception {
+        Properties props = new Properties();
+        props.setProperty("file", "${file:utf8:/etc/passwd}");
+        props.setProperty("hax", "${script:Groovy:jenkins.model.Jenkins.get().systemMessage = 'pwn3d'}");
+        File textFile = temp.newFile();
+        try (FileWriter f = new FileWriter(textFile)) {
+            props.store(f, "Pipeline test");
+        }
+
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition(
+                "node('slaves') {\n" +
+                        "  def props = readProperties interpolate: true, file: '" + separatorsToSystemEscaped(textFile.getAbsolutePath()) + "'\n" +
+                        "  assert props['file'] == '${file:utf8:/etc/passwd}'\n" +
+                        "  assert props['hax'] == '${script:Groovy:jenkins.model.Jenkins.get().systemMessage = \\'pwn3d\\'}'\n" +
+                        "}", true));
+        j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+        assertNotEquals("pwn3d", j.jenkins.getSystemMessage());
+    }
+
+    @Issue("SECURITY-2949")
+    @Test
+    public void safeInterpolatorsDoInterpolate() throws Exception {
+        Properties props = new Properties();
+        props.setProperty("urld", "${urlDecoder:Hello+World%21}");
+        props.setProperty("urle", "${urlEncoder:Hello World!}");
+        props.setProperty("base64d", "${base64Decoder:SGVsbG9Xb3JsZCE=}");
+        props.setProperty("base64e", "${base64Encoder:HelloWorld!}");
+        File textFile = temp.newFile();
+        try (FileWriter f = new FileWriter(textFile)) {
+            props.store(f, "Pipeline test");
+        }
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition(
+                "node('slaves') {\n" +
+                        "  def props = readProperties interpolate: true, file: '" + separatorsToSystemEscaped(textFile.getAbsolutePath()) + "'\n" +
+                        "  assert props['base64d'] == 'HelloWorld!'\n" +
+                        "  assert props['base64e'] == 'SGVsbG9Xb3JsZCE='\n" +
+                        "  assert props['urld'] == 'Hello World!'\n" +
+                        "  assert props['urle'] == 'Hello+World%21'\n" +
+                        "}", true));
+        j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+    }
+
+    @Issue("SECURITY-2949")
+    @Test
+    public void customLookupsOverrideDefaultInterpolators() throws Exception {
+        CUSTOM_PREFIX_INTERPOLATOR_LOOKUPS = "script,base64_encoder";
+
+        Properties props = new Properties();
+        props.setProperty("hax", "${script:Groovy:jenkins.model.Jenkins.get().systemMessage = 'pwn3d'}");
+        props.setProperty("base64d", "${base64Decoder:SGVsbG9Xb3JsZCE=}");
+        props.setProperty("base64e", "${base64Encoder:HelloWorld!}");
+        File textFile = temp.newFile();
+        try (FileWriter f = new FileWriter(textFile)) {
+            props.store(f, "Pipeline test");
+        }
+
+        WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition(
+                "node('slaves') {\n" +
+                        "  def props = readProperties interpolate: true, file: '" + separatorsToSystemEscaped(textFile.getAbsolutePath()) + "'\n" +
+                        "  assert props['base64d'] == '${base64Decoder:SGVsbG9Xb3JsZCE=}'\n" +
+                        "  assert props['base64e'] == 'SGVsbG9Xb3JsZCE='\n" +
+                        "}", true));
+        j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+        assertEquals("pwn3d", j.jenkins.getSystemMessage());
+    }
+}