CVE-2017-2603: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: Jenkins before versions 2.44, 2.32.2 is vulnerable to a user data leak in disconnected agents' config.xml API. This could leak sensitive data such as API tokens (SECURITY-362).
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 3.5
Published: 2018-05-15T21:29Z
Repository: https://github.com/jenkinsci/jenkins
Language: XML
Files changed: core/src/main/java/hudson/slaves/OfflineCause.java, test/src/test/java/hudson/model/ComputerTest.java, test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/config.xml, test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/nodes/deserialized/config.xml, test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/users/username/config.xml
Commit date: 2017-01-23 12:55:54 -0500
Commit message: Merge pull request #80 from jenkinsci-cert/SECURITY-362

[SECURITY-362] Do not persist User in OfflineCause.UserCause
Diff: diff --git a/core/src/main/java/hudson/slaves/OfflineCause.java b/core/src/main/java/hudson/slaves/OfflineCause.java
index 41319f01e92..3b94e99cd9f 100644
--- a/core/src/main/java/hudson/slaves/OfflineCause.java
+++ b/core/src/main/java/hudson/slaves/OfflineCause.java
@@ -22,20 +22,22 @@
  * THE SOFTWARE.
  */
 
 package hudson.slaves;
 
-import jenkins.model.Jenkins;
 import hudson.Functions;
 import hudson.model.Computer;
 import hudson.model.User;
 
 import org.jvnet.localizer.Localizable;
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.stapler.export.Exported;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import java.io.ObjectStreamException;
+import java.util.Collections;
 import java.util.Date;
 
 /**
  * Represents a cause that puts a {@linkplain Computer#isOffline() computer offline}.
  *
@@ -126,25 +128,53 @@ public abstract class OfflineCause {
         }
     }
 
     /**
      * Taken offline by user.
+     *
      * @since 1.551
      */
     public static class UserCause extends SimpleOfflineCause {
-        private final User user;
-
-        public UserCause(User user, String message) {
-            super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(
-                    user!=null ? user.getId() : Jenkins.ANONYMOUS.getName(),
+        @Deprecated
+        private transient User user;
+        // null when unknown
+        private /*final*/ @CheckForNull String userId;
+
+        public UserCause(@CheckForNull User user, @CheckForNull String message) {
+            this(
+                    user != null ? user.getId() : null,
                     message != null ? " : " + message : ""
-            ));
-            this.user = user;
+            );
+        }
+
+        private UserCause(String userId, String message) {
+            super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(userId, message));
+            this.userId = userId;
         }
 
         public User getUser() {
-            return user;
+            return userId == null
+                    ? User.getUnknown()
+                    : User.getById(userId, true)
+            ;
+        }
+
+        // Storing the User in a filed was a mistake, switch to userId
+        @SuppressWarnings("deprecation")
+        private Object readResolve() throws ObjectStreamException {
+            if (user != null) {
+                String id = user.getId();
+                if (id != null) {
+                    userId = id;
+                } else {
+                    // The user field is not properly deserialized so id may be missing. Look the user up by fullname
+                    User user = User.get(this.user.getFullName(), true, Collections.emptyMap());
+                    userId = user.getId();
+                }
+                this.user = null;
+            }
+            return this;
         }
     }
 
     public static class ByCLI extends UserCause {
         @Exported
diff --git a/test/src/test/java/hudson/model/ComputerTest.java b/test/src/test/java/hudson/model/ComputerTest.java
index b58b00f45e5..ffe67ddb15d 100644
--- a/test/src/test/java/hudson/model/ComputerTest.java
+++ b/test/src/test/java/hudson/model/ComputerTest.java
@@ -21,20 +21,26 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
 package hudson.model;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.*;
 
+
 import java.io.File;
 
+import com.gargoylesoftware.htmlunit.xml.XmlPage;
+import hudson.slaves.OfflineCause;
 import jenkins.model.Jenkins;
 import hudson.slaves.DumbSlave;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.LocalData;
 
 public class ComputerTest {
 
     @Rule public JenkinsRule j = new JenkinsRule();
 
@@ -50,6 +56,29 @@ public class ComputerTest {
         assertFalse("Slave log should be deleted", logFile.exists());
         assertFalse("Slave log directory should be deleted", logFile.getParentFile().exists());
 
         assertTrue("Slave log should be kept", keep.toComputer().getLogFile().exists());
     }
+
+    @Test
+    public void doNotShowUserDetailsInOfflineCause() throws Exception {
+        DumbSlave slave = j.createOnlineSlave();
+        final Computer computer = slave.toComputer();
+        computer.setTemporarilyOffline(true, new OfflineCause.UserCause(User.get("username"), "msg"));
+        verifyOfflineCause(computer);
+    }
+
+    @Test @LocalData
+    public void removeUserDetailsFromOfflineCause() throws Exception {
+        Computer computer = j.jenkins.getComputer("deserialized");
+        verifyOfflineCause(computer);
+    }
+
+    private void verifyOfflineCause(Computer computer) throws Exception {
+        XmlPage page = j.createWebClient().goToXml("computer/" + computer.getName() + "/config.xml");
+        String content = page.getWebResponse().getContentAsString("UTF-8");
+        assertThat(content, containsString("temporaryOfflineCause"));
+        assertThat(content, containsString("<userId>username</userId>"));
+        assertThat(content, not(containsString("ApiTokenProperty")));
+        assertThat(content, not(containsString("apiToken")));
+    }
 }
diff --git a/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/config.xml b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/config.xml
new file mode 100644
index 00000000000..1a1cd9abbb2
--- /dev/null
+++ b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/config.xml
@@ -0,0 +1,34 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<hudson>
+  <disabledAdministrativeMonitors/>
+  <version>1.0</version>
+  <numExecutors>2</numExecutors>
+  <mode>NORMAL</mode>
+  <useSecurity>true</useSecurity>
+  <authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
+  <securityRealm class="hudson.security.SecurityRealm$None"/>
+  <disableRememberMe>false</disableRememberMe>
+  <projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
+  <workspaceDir>${JENKINS_HOME}/workspace/${ITEM_FULLNAME}</workspaceDir>
+  <buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
+  <jdks/>
+  <viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
+  <myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
+  <clouds/>
+  <scmCheckoutRetryCount>0</scmCheckoutRetryCount>
+  <views>
+    <hudson.model.AllView>
+      <owner class="hudson" reference="../../.."/>
+      <name>All</name>
+      <filterExecutors>false</filterExecutors>
+      <filterQueue>false</filterQueue>
+      <properties class="hudson.model.View$PropertyList"/>
+    </hudson.model.AllView>
+  </views>
+  <primaryView>All</primaryView>
+  <slaveAgentPort>0</slaveAgentPort>
+  <label></label>
+  <nodeProperties/>
+  <globalNodeProperties/>
+  <noUsageStatistics>true</noUsageStatistics>
+</hudson>
diff --git a/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/nodes/deserialized/config.xml b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/nodes/deserialized/config.xml
new file mode 100644
index 00000000000..b093f82a5cf
--- /dev/null
+++ b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/nodes/deserialized/config.xml
@@ -0,0 +1,56 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<slave>
+    <temporaryOfflineCause class="hudson.slaves.OfflineCause$UserCause">
+        <timestamp>1479196265920</timestamp>
+        <description>
+            <holder>
+                <owner>hudson.slaves.Messages</owner>
+            </holder>
+            <key>SlaveComputer.DisconnectedBy</key>
+            <args>
+                <string>username</string>
+                <string> : msg</string>
+            </args>
+        </description>
+        <user>
+            <fullName>username</fullName>
+            <properties>
+                <jenkins.security.ApiTokenProperty>
+                    <apiToken>wPfVKd4HGJzRoEpazbTu35nXXfI34cguPjm+5JPO7pZDFLFgpFLviQsS3NdJndax</apiToken>
+                </jenkins.security.ApiTokenProperty>
+                <hudson.tasks.Mailer_-UserProperty/>
+                <hudson.model.MyViewsProperty>
+                    <views>
+                        <hudson.model.AllView>
+                            <owner class="hudson.model.MyViewsProperty" reference="../../.."/>
+                            <name>All</name>
+                            <filterExecutors>false</filterExecutors>
+                            <filterQueue>false</filterQueue>
+                            <properties class="hudson.model.View$PropertyList"/>
+                        </hudson.model.AllView>
+                    </views>
+                </hudson.model.MyViewsProperty>
+                <hudson.model.PaneStatusProperties>
+                    <collapsed/>
+                </hudson.model.PaneStatusProperties>
+                <hudson.search.UserSearchProperty>
+                    <insensitiveSearch>false</insensitiveSearch>
+                </hudson.search.UserSearchProperty>
+            </properties>
+        </user>
+    </temporaryOfflineCause>
+    <name>deserialized</name>
+    <description>dummy</description>
+    <remoteFS>...</remoteFS>
+    <numExecutors>1</numExecutors>
+    <mode>NORMAL</mode>
+    <retentionStrategy class="hudson.slaves.RetentionStrategy$2">
+        <DESCRIPTOR>
+            <outer-class reference="../.."/>
+        </DESCRIPTOR>
+    </retentionStrategy>
+    <launcher/>
+    <label></label>
+    <nodeProperties/>
+    <userId>SYSTEM</userId>
+</slave>
diff --git a/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/users/username/config.xml b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/users/username/config.xml
new file mode 100644
index 00000000000..9b55a52743c
--- /dev/null
+++ b/test/src/test/resources/hudson/model/ComputerTest/removeUserDetailsFromOfflineCause/users/username/config.xml
@@ -0,0 +1,38 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<user>
+  <fullName>username</fullName>
+  <id>username</id>
+  <description></description>
+  <properties>
+    <jenkins.security.ApiTokenProperty>
+      <apiToken>qykj8q6EqvMg9LPu+lCqLiXBZvEVdCTWoYJwmicXgH+yh1ZUm85iHe29grd+g3QG</apiToken>
+    </jenkins.security.ApiTokenProperty>
+    <com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty plugin="credentials@1.18">
+      <domainCredentialsMap class="hudson.util.CopyOnWriteMap$Hash"/>
+    </com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty>
+    <hudson.tasks.Mailer_-UserProperty>
+      <emailAddress></emailAddress>
+    </hudson.tasks.Mailer_-UserProperty>
+    <hudson.model.MyViewsProperty>
+      <primaryViewName></primaryViewName>
+      <views>
+        <hudson.model.AllView>
+          <owner class="hudson.model.MyViewsProperty" reference="../../.."/>
+          <name>All</name>
+          <filterExecutors>false</filterExecutors>
+          <filterQueue>false</filterQueue>
+          <properties class="hudson.model.View$PropertyList"/>
+        </hudson.model.AllView>
+      </views>
+    </hudson.model.MyViewsProperty>
+    <hudson.model.PaneStatusProperties>
+      <collapsed/>
+    </hudson.model.PaneStatusProperties>
+    <org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl>
+      <authorizedKeys></authorizedKeys>
+    </org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl>
+    <hudson.search.UserSearchProperty>
+      <insensitiveSearch>false</insensitiveSearch>
+    </hudson.search.UserSearchProperty>
+  </properties>
+</user>

Vulnerable code: import jenkins.model.Jenkins;
        private final User user;

        public UserCause(User user, String message) {
            super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(
                    user!=null ? user.getId() : Jenkins.ANONYMOUS.getName(),
            ));
            this.user = user;
            return user;
Fixed code: import javax.annotation.CheckForNull;
import java.io.ObjectStreamException;
import java.util.Collections;
     *
        @Deprecated
        private transient User user;
        // null when unknown
        private /*final*/ @CheckForNull String userId;

        public UserCause(@CheckForNull User user, @CheckForNull String message) {
            this(
                    user != null ? user.getId() : null,
            );
        }

        private UserCause(String userId, String message) {
            super(hudson.slaves.Messages._SlaveComputer_DisconnectedBy(userId, message));
            this.userId = userId;
            return userId == null
                    ? User.getUnknown()
                    : User.getById(userId, true)
            ;
        }

        // Storing the User in a filed was a mistake, switch to userId
        @SuppressWarnings("deprecation")
        private Object readResolve() throws ObjectStreamException {
            if (user != null) {
                String id = user.getId();
                if (id != null) {
                    userId = id;
                } else {
                    // The user field is not properly deserialized so id may be missing. Look the user up by fullname
                    User user = User.get(this.user.getFullName(), true, Collections.emptyMap());
                    userId = user.getId();
                }
                this.user = null;
            }
            return this;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;

import com.gargoylesoftware.htmlunit.xml.XmlPage;
import hudson.slaves.OfflineCause;
import org.jvnet.hudson.test.recipes.LocalData;

    @Test
    public void doNotShowUserDetailsInOfflineCause() throws Exception {
        DumbSlave slave = j.createOnlineSlave();
        final Computer computer = slave.toComputer();
        computer.setTemporarilyOffline(true, new OfflineCause.UserCause(User.get("username"), "msg"));
        verifyOfflineCause(computer);
    }

    @Test @LocalData
    public void removeUserDetailsFromOfflineCause() throws Exception {
        Computer computer = j.jenkins.getComputer("deserialized");
        verifyOfflineCause(computer);
    }

    private void verifyOfflineCause(Computer computer) throws Exception {
        XmlPage page = j.createWebClient().goToXml("computer/" + computer.getName() + "/config.xml");
        String content = page.getWebResponse().getContentAsString("UTF-8");
        assertThat(content, containsString("temporaryOfflineCause"));
        assertThat(content, containsString("<userId>username</userId>"));
        assertThat(content, not(containsString("ApiTokenProperty")));
        assertThat(content, not(containsString("apiToken")));
    }
<?xml version='1.0' encoding='UTF-8'?>
<hudson>
  <disabledAdministrativeMonitors/>
  <version>1.0</version>
  <numExecutors>2</numExecutors>
  <mode>NORMAL</mode>
  <useSecurity>true</useSecurity>
  <authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
  <securityRealm class="hudson.security.SecurityRealm$None"/>
  <disableRememberMe>false</disableRememberMe>
  <projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
  <workspaceDir>${JENKINS_HOME}/workspace/${ITEM_FULLNAME}</workspaceDir>
  <buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
  <jdks/>
  <viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
  <myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
  <clouds/>
  <scmCheckoutRetryCount>0</scmCheckoutRetryCount>
  <views>
    <hudson.model.AllView>
      <owner class="hudson" reference="../../.."/>
      <name>All</name>
      <filterExecutors>false</filterExecutors>
      <filterQueue>false</filterQueue>
      <properties class="hudson.model.View$PropertyList"/>
    </hudson.model.AllView>
  </views>
  <primaryView>All</primaryView>
  <slaveAgentPort>0</slaveAgentPort>
  <label></label>
  <nodeProperties/>
  <globalNodeProperties/>
  <noUsageStatistics>true</noUsageStatistics>
</hudson>
<?xml version='1.0' encoding='UTF-8'?>
<slave>
    <temporaryOfflineCause class="hudson.slaves.OfflineCause$UserCause">
        <timestamp>1479196265920</timestamp>
        <description>
            <holder>
                <owner>hudson.slaves.Messages</owner>
            </holder>
            <key>SlaveComputer.DisconnectedBy</key>
            <args>
                <string>username</string>
                <string> : msg</string>
            </args>
        </description>
        <user>
            <fullName>username</fullName>
            <properties>
                <jenkins.security.ApiTokenProperty>
                    <apiToken>wPfVKd4HGJzRoEpazbTu35nXXfI34cguPjm+5JPO7pZDFLFgpFLviQsS3NdJndax</apiToken>
                </jenkins.security.ApiTokenProperty>
                <hudson.tasks.Mailer_-UserProperty/>
                <hudson.model.MyViewsProperty>
                    <views>
                        <hudson.model.AllView>
                            <owner class="hudson.model.MyViewsProperty" reference="../../.."/>
                            <name>All</name>
                            <filterExecutors>false</filterExecutors>
                            <filterQueue>false</filterQueue>
                            <properties class="hudson.model.View$PropertyList"/>
                        </hudson.model.AllView>
                    </views>
                </hudson.model.MyViewsProperty>
                <hudson.model.PaneStatusProperties>
                    <collapsed/>
                </hudson.model.PaneStatusProperties>
                <hudson.search.UserSearchProperty>
                    <insensitiveSearch>false</insensitiveSearch>
                </hudson.search.UserSearchProperty>
            </properties>
        </user>
    </temporaryOfflineCause>
    <name>deserialized</name>
    <description>dummy</description>
    <remoteFS>...</remoteFS>
    <numExecutors>1</numExecutors>
    <mode>NORMAL</mode>
    <retentionStrategy class="hudson.slaves.RetentionStrategy$2">
        <DESCRIPTOR>
            <outer-class reference="../.."/>
        </DESCRIPTOR>
    </retentionStrategy>
    <launcher/>
    <label></label>
    <nodeProperties/>
    <userId>SYSTEM</userId>
</slave>
<?xml version='1.0' encoding='UTF-8'?>
<user>
  <fullName>username</fullName>
  <id>username</id>
  <description></description>
  <properties>
    <jenkins.security.ApiTokenProperty>
      <apiToken>qykj8q6EqvMg9LPu+lCqLiXBZvEVdCTWoYJwmicXgH+yh1ZUm85iHe29grd+g3QG</apiToken>
    </jenkins.security.ApiTokenProperty>
    <com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty plugin="credentials@1.18">
      <domainCredentialsMap class="hudson.util.CopyOnWriteMap$Hash"/>
    </com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty>
    <hudson.tasks.Mailer_-UserProperty>
      <emailAddress></emailAddress>
    </hudson.tasks.Mailer_-UserProperty>
    <hudson.model.MyViewsProperty>
      <primaryViewName></primaryViewName>
      <views>
        <hudson.model.AllView>
          <owner class="hudson.model.MyViewsProperty" reference="../../.."/>
          <name>All</name>
          <filterExecutors>false</filterExecutors>
          <filterQueue>false</filterQueue>
          <properties class="hudson.model.View$PropertyList"/>
        </hudson.model.AllView>
      </views>
    </hudson.model.MyViewsProperty>
    <hudson.model.PaneStatusProperties>
      <collapsed/>
    </hudson.model.PaneStatusProperties>
    <org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl>
      <authorizedKeys></authorizedKeys>
    </org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl>
    <hudson.search.UserSearchProperty>
      <insensitiveSearch>false</insensitiveSearch>
    </hudson.search.UserSearchProperty>
  </properties>
</user>