HuntersHunters
Sign inStart free
Security context
Medium· 4.3GHSA-279f-qwgh-h5mp CVE-2023-43494Published Sep 20, 2023

Jenkins does not exclude sensitive build variables from search

Research this vulnerability

Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.

Affected versions

2.50 → fixed in 2.414.22.415 → fixed in 2.424

Details

Jenkins allows filtering builds in the build history widget by specifying an expression that searches for matching builds by name, description, parameter values, etc. Jenkins 2.50 through 2.423 (both inclusive), LTS 2.60.1 through 2.414.1 (both inclusive) does not exclude sensitive build variables (e.g., password parameter values) from this search. This allows attackers with Item/Read permission to obtain values of sensitive variables used in builds by iteratively testing different characters until the correct sequence is discovered. Jenkins 2.424, LTS 2.414.2 excludes sensitive variables from this search.

The fix

[SECURITY-3261]

Daniel Beck· Sep 6, 2023, 11:17 AM+133−2b8ac8cd4c5
core/src/main/java/jenkins/widgets/HistoryPageFilter.java+4 −2
@@ -42,6 +42,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import jenkins.model.queue.QueueItem;
  
 /**
@@ -371,8 +372,9 @@ private boolean fitsSearchString(Object data) {
  
     private boolean fitsSearchBuildVariables(AbstractBuild<?, ?> runAsBuild) {
         Map<String, String> buildVariables = runAsBuild.getBuildVariables();
-        for (String paramsValues : buildVariables.values()) {
-            if (fitsSearchString(paramsValues)) {
+        Set<String> sensitiveBuildVariables = runAsBuild.getSensitiveBuildVariables();
+        for (Map.Entry<String, String> param : buildVariables.entrySet()) {
+            if (!sensitiveBuildVariables.contains(param.getKey()) && fitsSearchString(param.getValue())) {
                 return true;
             }
         }
core/src/test/java/jenkins/widgets/HistoryPageFilterTest.java+7 −0
@@ -43,6 +43,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 import jenkins.model.queue.QueueItem;
 import org.junit.Assert;
@@ -485,6 +486,7 @@ private static class MockBuild extends Build<FreeStyleProject, FreeStyleBuild> {
         private final int buildNumber;
  
         private Map<String, String> buildVariables = Collections.emptyMap();
+        private Set<String> sensitiveBuildVariables = Collections.emptySet();
  
         private MockBuild(int buildNumber) {
             super(Mockito.mock(FreeStyleProject.class), Mockito.mock(Calendar.class));
@@ -501,6 +503,11 @@ public Map<String, String> getBuildVariables() {
             return buildVariables;
         }
  
+        @Override
+        public Set<String> getSensitiveBuildVariables() {
+            return sensitiveBuildVariables; // TODO This is never actually set (bad Mock), actual test in test harness
+        }
+ 
         MockBuild withBuildVariables(Map<String, String> buildVariables) {
             this.buildVariables = buildVariables;
             return this;
test/src/test/java/jenkins/widgets/HistoryPageFilterTest.java+122 −0
@@ -0,0 +1,122 @@
+package jenkins.widgets;
+ 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+ 
+import hudson.model.AbstractBuild;
+import hudson.model.FreeStyleBuild;
+import hudson.model.FreeStyleProject;
+import hudson.model.ParametersAction;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.PasswordParameterDefinition;
+import hudson.model.StringParameterDefinition;
+import hudson.tasks.BuildWrapper;
+import hudson.util.Secret;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+ 
+ 
+public class HistoryPageFilterTest {
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+ 
+    @Test
+    public void doNotFindSensitiveBuildParams() throws Exception {
+        final FreeStyleProject freeStyleProject = j.createFreeStyleProject();
+        final PasswordParameterDefinition passwordParameterDefinition = new PasswordParameterDefinition("password", Secret.fromString("t0ps3cr3t"), "description");
+        final StringParameterDefinition stringParameterDefinition = new StringParameterDefinition("key", "value", "desc");
+        freeStyleProject.addProperty(new ParametersDefinitionProperty(passwordParameterDefinition, stringParameterDefinition));
+        final FreeStyleBuild build1 = j.buildAndAssertSuccess(freeStyleProject);
+        final FreeStyleBuild build2 = j.waitForCompletion(Objects.requireNonNull(freeStyleProject.scheduleBuild2(
+                0,
+                new ParametersAction(
+                        passwordParameterDefinition.createValue("p4ssw0rd"),
+                        stringParameterDefinition.createValue("value123"))))
+                .waitForStart());
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("value");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build2, build1));
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("t0p");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs, empty());
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("value123");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build2));
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("p4ss");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs, empty());
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+    }
+ 
+    @Test
+    public void doNotFindSensitiveBuildWrapperVars() throws Exception {
+        final FreeStyleProject freeStyleProject = j.createFreeStyleProject();
+        freeStyleProject.getBuildWrappersList().add(new BuildWrapperWithSomeSensitiveVars(Map.of("key1", "value123", "key2", "value234", "key3", "s3cr3t"), Set.of("key3")));
+        final FreeStyleBuild build = j.buildAndAssertSuccess(freeStyleProject);
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("value");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build));
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("value123");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs.stream().map(HistoryPageEntry::getEntry).collect(Collectors.toList()), contains(build));
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+        {
+            final HistoryPageFilter<FreeStyleBuild> historyPageFilter = new HistoryPageFilter<>(30);
+            historyPageFilter.setSearchString("s3cr3t");
+            historyPageFilter.add(freeStyleProject.getBuilds());
+            assertThat(historyPageFilter.runs, empty());
+            assertThat(historyPageFilter.queueItems, empty());
+        }
+    }
+ 
+    private static class BuildWrapperWithSomeSensitiveVars extends BuildWrapper {
+        private final Map<String, String> variables;
+        private final Set<String> sensitiveVariables;
+ 
+        private BuildWrapperWithSomeSensitiveVars(Map<String, String> variables, Set<String> sensitiveVariables) {
+ 
+            this.variables = new HashMap<>(variables);
+            this.sensitiveVariables = new HashSet<>(sensitiveVariables);
+        }
+ 
+        @Override
+        public void makeBuildVariables(AbstractBuild build, Map<String, String> variables) {
+            variables.putAll(this.variables);
+        }
+ 
+        @Override
+        public void makeSensitiveBuildVariables(AbstractBuild build, Set<String> sensitiveVariables) {
+            sensitiveVariables.addAll(this.sensitiveVariables);
+        }
+    }
+}

References