Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-22665

BuildPipelineView.MyUserIdCause stores entire hudson.model.User

      Since MyUserIdCause.user is not transient, the entire User object is serialized to a build record as per $JENKINS_HOME/users/*/config.xml, including dangerous things like a customized API token and credentials.

      And the class is not static, so it serializes a reference to the BuildPipelineView mentioning it.

      Example:

      <?xml version='1.0' encoding='UTF-8'?>
      <build>
        <actions>
          ...
          <hudson.model.CauseAction>
            <causes>
              <au.com.centrumsystems.hudson.plugin.buildpipeline.BuildPipelineView_-MyUserIdCause plugin="build-pipeline-plugin@1.3.3">
                <userId>person@somewhere.com</userId>
                <user>
                  <fullName>Some Person</fullName>
                  <properties>
                    <jenkins.security.ApiTokenProperty>
                      <apiToken>OOPS!</apiToken>
                    </jenkins.security.ApiTokenProperty>
                    <com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty plugin="credentials@1.9.3">
                      <domainCredentialsMap class="hudson.util.CopyOnWriteMap$Hash">
                        <entry>
                          ...
                        </entry>
                      </domainCredentialsMap>
                    </com.cloudbees.plugins.credentials.UserCredentialsProvider_-UserCredentialsProperty>
                    <hudson.model.MyViewsProperty>
                      <views>
                        ...
                      </views>
                    </hudson.model.MyViewsProperty>
                    <hudson.plugins.openid.OpenIdUserProperty plugin="openid@2.3">
                      <identifiers>
                        <string>OOPS!</string>
                      </identifiers>
                    </hudson.plugins.openid.OpenIdUserProperty>
                    ...
                  </properties>
                </user>
                <outer-class reference="../user/properties/hudson.model.MyViewsProperty/views/au.com.centrumsystems.hudson.plugin.buildpipeline.BuildPipelineView[10]"/>
              </au.com.centrumsystems.hudson.plugin.buildpipeline.BuildPipelineView_-MyUserIdCause>
            </causes>
          </hudson.model.CauseAction>
          ...
        </actions>
        ...
      </build>
      

      A Cause must be a static class with a small serial form. In this case you need only a String userId field; use User.get to retrieve the live object on demand.

      (Or just use the standard UserIdCause. It is not clear why you felt the need to subclass that.)

          [JENKINS-22665] BuildPipelineView.MyUserIdCause stores entire hudson.model.User

          Kim Nyhjem added a comment -

          This is a serious security breach.

          It not only affects config.xml, but also build.xml's, which means a lot of those dangerous (and very bulky if you have a lot of nested views) elements out there.

          Please upvote.

          Kim Nyhjem added a comment - This is a serious security breach. It not only affects config.xml, but also build.xml's, which means a lot of those dangerous (and very bulky if you have a lot of nested views) elements out there. Please upvote.

          Daniel Beck added a comment -

          JENKINS-24994 suggests disallowing Causes like completely by throwing if the class is anonymous.

          Daniel Beck added a comment - JENKINS-24994 suggests disallowing Causes like completely by throwing if the class is anonymous.

          Jesse Glick added a comment -

          This class is not anonymous. It is not static, so it gets a bogus reference to the BuildPipelineView.this, but that just makes for messy XML; fixing that would not fix the security hole.

          Jesse Glick added a comment - This class is not anonymous. It is not static , so it gets a bogus reference to the BuildPipelineView.this , but that just makes for messy XML; fixing that would not fix the security hole.

          Patrik Boström added a comment - Created PR with a proposed fix: https://github.com/jenkinsci/build-pipeline-plugin/pull/64

          Code changed in jenkins
          User: Patrik Boström
          Path:
          src/main/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineView.java
          src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest.java
          src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/trigger/BuildPipelineTriggerTest.java
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/config.xml
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/builds/2015-01-06_09-41-20/build.xml
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/config.xml
          http://jenkins-ci.org/commit/build-pipeline-plugin/bd77518bb3b9220f979f7906b210b2dd2225bada
          Log:
          [FIXED JENKINS-22665] [FIXED JENKINS-19755] Changed MyUserIdCause to not include the whole User object serialized.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Patrik Boström Path: src/main/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineView.java src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest.java src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/trigger/BuildPipelineTriggerTest.java src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/config.xml src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/builds/2015-01-06_09-41-20/build.xml src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/config.xml http://jenkins-ci.org/commit/build-pipeline-plugin/bd77518bb3b9220f979f7906b210b2dd2225bada Log: [FIXED JENKINS-22665] [FIXED JENKINS-19755] Changed MyUserIdCause to not include the whole User object serialized.

          Code changed in jenkins
          User: Kanstantsin Shautsou
          Path:
          src/main/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineView.java
          src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest.java
          src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/trigger/BuildPipelineTriggerTest.java
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/config.xml
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/builds/2015-01-06_09-41-20/build.xml
          src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/config.xml
          http://jenkins-ci.org/commit/build-pipeline-plugin/7e03b73fa2f1e134ebc6c904591ddbe494be478a
          Log:
          Merge pull request #64 from patbos/JENKINS-22665

          [FIXED JENKINS-22665] Fixes for JENKINS-22665 and JENKINS-19755

          Compare: https://github.com/jenkinsci/build-pipeline-plugin/compare/25ccbeff03aa...7e03b73fa2f1

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kanstantsin Shautsou Path: src/main/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineView.java src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest.java src/test/java/au/com/centrumsystems/hudson/plugin/buildpipeline/trigger/BuildPipelineTriggerTest.java src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/config.xml src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/builds/2015-01-06_09-41-20/build.xml src/test/resources/au/com/centrumsystems/hudson/plugin/buildpipeline/BuildPipelineViewTest/testMyUserIdCauseConversion/jobs/B/config.xml http://jenkins-ci.org/commit/build-pipeline-plugin/7e03b73fa2f1e134ebc6c904591ddbe494be478a Log: Merge pull request #64 from patbos/ JENKINS-22665 [FIXED JENKINS-22665] Fixes for JENKINS-22665 and JENKINS-19755 Compare: https://github.com/jenkinsci/build-pipeline-plugin/compare/25ccbeff03aa...7e03b73fa2f1

            Unassigned Unassigned
            jglick Jesse Glick
            Votes:
            8 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: