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

Field entries for hudson.scm.EditType fields in "jenkins-whitelist" must be static

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Critical Critical
    • script-security-plugin
    • None
    • Jenkins 2.46, Script Security Plugin 1.26
    • script-security 1218.v39ca_7f7ed0a_c

      Since hudson.scm.EditType implementation has not changed in the last 10 years, I think either the white listing never ever worked, or (less likely?) the white listing syntax changed and in the past there was no differentiation between static and non-static fields.

      Anyway, could you please change the following in "org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/jenkins-whitelist":

      • From old:
        field hudson.scm.EditType ADD
        field hudson.scm.EditType DELETE
        field hudson.scm.EditType EDIT
        
      • => New:
        staticField hudson.scm.EditType ADD
        staticField hudson.scm.EditType DELETE
        staticField hudson.scm.EditType EDIT
        

          [JENKINS-42214] Field entries for hudson.scm.EditType fields in "jenkins-whitelist" must be static

          I apologize for the direct assignment to you, Jesse, but I dared to do that, because (a) you seem to be the official maintainer of the plugin (see https://wiki.jenkins-ci.org/display/JENKINS/Script+Security+Plugin) and (b) I hope this is a low hanging fruit

          Reinhold Füreder added a comment - I apologize for the direct assignment to you, Jesse, but I dared to do that, because (a) you seem to be the official maintainer of the plugin (see https://wiki.jenkins-ci.org/display/JENKINS/Script+Security+Plugin ) and (b) I hope this is a low hanging fruit

          Jesse Glick added a comment -

          The whitelist validation code must have been buggy to accept this to begin with, so a proper fix involves code changes.

          Jesse Glick added a comment - The whitelist validation code must have been buggy to accept this to begin with, so a proper fix involves code changes.

          Well, I claim that the current entries are just wrong then. And since I am actually needing and actively using such static field whitelist entries in other contexts (e.g. "staticField hudson.model.Result FAILURE", cf. JENKINS-35352) with success, I was pretty confident that this would help here as well...

          Reinhold Füreder added a comment - Well, I claim that the current entries are just wrong then. And since I am actually needing and actively using such static field whitelist entries in other contexts (e.g. " staticField hudson.model.Result FAILURE ", cf. JENKINS-35352 ) with success, I was pretty confident that this would help here as well...

          Jesse Glick added a comment -

          I claim that the current entries are just wrong

          I was not disputing that, only pointing out that the proper fix also involves fixing MethodSignature.exists so that no such mistake could happen again.

          Jesse Glick added a comment - I claim that the current entries are just wrong I was not disputing that, only pointing out that the proper fix also involves fixing MethodSignature.exists so that no such mistake could happen again.

          Oh, okay, I am sorry for my misunderstanding and must admit that your approach (fixing MethodSignature.exists so that no such mistake could happen again) is the right one...

          Reinhold Füreder added a comment - Oh, okay, I am sorry for my misunderstanding and must admit that your approach ( fixing MethodSignature.exists so that no such mistake could happen again ) is the right one...

          Jesse Glick added a comment -

          Ah…I am already checking static in MethodSignature and StaticMethodSignature, but not FieldSignature or StaticFieldSignature.

          Jesse Glick added a comment - Ah…I am already checking static in MethodSignature and StaticMethodSignature , but not FieldSignature or StaticFieldSignature .

          Devin Nusbaum added a comment -

          I opened a PR to fix this issue (just happened to notice it while looking through other issues): https://github.com/jenkinsci/script-security-plugin/pull/298.

          Devin Nusbaum added a comment - I opened a PR to fix this issue (just happened to notice it while looking through other issues):  https://github.com/jenkinsci/script-security-plugin/pull/298 .

          dnusbaum Thanks for taking over!

          Actually I am only responding to your comment to let you know that the Jenkins JIRA email notification contains links to https://url1399.jenkins.io (instead of e,g, https://issues.jenkins-ci.org/browse/JENKINS-42214 like I think in the past) which has HSTS security issues: and this is a rather very new problem, I think...

          Reinhold Füreder added a comment - dnusbaum Thanks for taking over! Actually I am only responding to your comment to let you know that the Jenkins JIRA email notification contains links to https://url1399.jenkins.io (instead of e,g, https://issues.jenkins-ci.org/browse/JENKINS-42214 like I think in the past) which has HSTS security issues: and this is a rather very new problem, I think...

          Devin Nusbaum added a comment -

          A fix for this issue was released in Script Security plugin version 1218.v39ca_7f7ed0a_c.

          Devin Nusbaum added a comment - A fix for this issue was released in Script Security plugin version 1218.v39ca_7f7ed0a_c .

          Hi dnusbaum , I have a problem with this fix i think. Can you confirm or infirm this introduce a regression : JENKINS-70108 Scripts not permitted to use new java.util.Properties - Jenkins Jira

          Cyril Pottiers added a comment - Hi dnusbaum , I have a problem with this fix i think. Can you confirm or infirm this introduce a regression : JENKINS-70108 Scripts not permitted to use new java.util.Properties - Jenkins Jira

            dnusbaum Devin Nusbaum
            reinholdfuereder Reinhold Füreder
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: