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

Modifying files in class directories can bypass approval in script-security (or class directories are accepted as classpaths)

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: script-security-plugin
    • Labels:
      None
    • Environment:
      Jenkins 1.509.4, script-security 1.5, Java 1.7.0_45, Windows 8 (64bit)
    • Similar Issues:

      Description

      • script-security 1.5 introduced "Additional classpath".
      • Those classpaths require administrators' approval.
      • Class directories are valid for "Additional classpath".
      • Once class directories are appoved, adding or replacing files in sub directories of those class directories no longer require approval.
      • This should allow users to use classes that administrators doesn't want to allow.

      Possible resolution:

      • Don't allow class directories for "Additional classpath"
        • This doesn't cause critical regressions as it is easy to create jar file from class directories.
      • When a class directory is specified, check all files in the class directory.
      • Leave this as a limitation.

      I'll add a test and send a pull request to see this behavior.

        Attachments

          Activity

          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Andres Rodriguez
          Path:
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java
          src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties
          src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java
          http://jenkins-ci.org/commit/script-security-plugin/ab0a6e1e14107f03fdd978c7148f6e1a0f79d50d
          Log:
          JENKINS-24399 Don't allow class directories any more.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andres Rodriguez Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java http://jenkins-ci.org/commit/script-security-plugin/ab0a6e1e14107f03fdd978c7148f6e1a0f79d50d Log: JENKINS-24399 Don't allow class directories any more.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Andres Rodriguez
          Path:
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
          http://jenkins-ci.org/commit/script-security-plugin/3c38ff1a33c5a860bf6fe36fe6a8394d33e524f3
          Log:
          JENKINS-24399 Refine URL criteria to identify class dirs.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andres Rodriguez Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java http://jenkins-ci.org/commit/script-security-plugin/3c38ff1a33c5a860bf6fe36fe6a8394d33e524f3 Log: JENKINS-24399 Refine URL criteria to identify class dirs.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java
          src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java
          src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties
          src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java
          src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java
          http://jenkins-ci.org/commit/script-security-plugin/6cb7ac90b708547878b9c61767bf294f46c8eb9e
          Log:
          Merge pull request #52 from andresrc/JENKINS-24399

          JENKINS-24399 Don't allow class directories any more.

          Compare: https://github.com/jenkinsci/script-security-plugin/compare/47ea2833a95d...6cb7ac90b708

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntryTest.java src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java http://jenkins-ci.org/commit/script-security-plugin/6cb7ac90b708547878b9c61767bf294f46c8eb9e Log: Merge pull request #52 from andresrc/ JENKINS-24399 JENKINS-24399 Don't allow class directories any more. Compare: https://github.com/jenkinsci/script-security-plugin/compare/47ea2833a95d...6cb7ac90b708
          Hide
          vimil vimil added a comment -

          Instead of not allowing classpath directories, is an enhancement to hash the contents of the directory better? I can provide a pull request for this enhancement if you think it won't cause other security issues

          Show
          vimil vimil added a comment - Instead of not allowing classpath directories, is an enhancement to hash the contents of the directory better? I can provide a pull request for this enhancement if you think it won't cause other security issues
          Hide
          jglick Jesse Glick added a comment -

          Fixed now I guess.

          Show
          jglick Jesse Glick added a comment - Fixed now I guess.

            People

            • Assignee:
              andresrc Andres Rodriguez
              Reporter:
              ikedam ikedam
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: