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

text parameter with newlines causing the default goal run

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • Jenkins 1.583, Windows, Java 7U72

      I have a required build-parameter "build_reason" of type text. Our users can insert some free-text for the build reason.

      If the parameter contains, for example, following input in the text-box:

      "first line
      second line"

      then jenkins calling maven in the following way:

      [testwebapp_svn_commit] $ D:\applications\prg\ApacheMaven\current-maven\bin\mvn.bat -Dbuild_reason_org=first line
      scond line -Dpart_svn_url=trunk/devopts/testprojects validate

      This causes maven to simple run the default goal and ignore all other parameters.
      It seems that the text parameter is not escaped correct.

      This problem does not apply only to maven .. it applies to all builds where such parameter is injected.

      • Waffel

          [JENKINS-25416] text parameter with newlines causing the default goal run

          Daniel Beck added a comment -

          Possibly. Could you try with ant as well?

          Daniel Beck added a comment - Possibly. Could you try with ant as well?

          Thomas Wabner added a comment -

          Yes, I have tried with ant ... more or less the same result:

          [testwebapp_svn_commit] $ cmd.exe /C '"D:\applications\prg\jenkinsSlave1\tools\hudson.tasks.Ant_AntInstallation\ant-1.9.3\bin\ant.bat -Ddeployment_reason=a
          b
          c -Dpart_svn_url=trunk/devopts/testprojects clean && exit %%ERRORLEVEL%%"'
          Buildfile: D:\applications\prg\jenkinsSlave1\workspace\test-webapp\testwebapp_svn_commit\build.xml

          test-offline:

          get-deps:
          ....

          the default goal is "package", but the goal configured is "clean" (which never tries to get the dependencies). With a one-liner it works.

          I have set for the parameter "deployment_reason" the content

          a
          b
          c

          I'm afraid that someone (the user) can also inject other commands into the command shell call and in the worst case hack or destroy the build agent (I have not started to check this ... but the escaping thing looks like a possible target)

          Thomas Wabner added a comment - Yes, I have tried with ant ... more or less the same result: [testwebapp_svn_commit] $ cmd.exe /C '"D:\applications\prg\jenkinsSlave1\tools\hudson.tasks.Ant_AntInstallation\ant-1.9.3\bin\ant.bat -Ddeployment_reason=a b c -Dpart_svn_url=trunk/devopts/testprojects clean && exit %%ERRORLEVEL%%"' Buildfile: D:\applications\prg\jenkinsSlave1\workspace\test-webapp\testwebapp_svn_commit\build.xml test-offline: get-deps: .... the default goal is "package", but the goal configured is "clean" (which never tries to get the dependencies). With a one-liner it works. I have set for the parameter "deployment_reason" the content a b c I'm afraid that someone (the user) can also inject other commands into the command shell call and in the worst case hack or destroy the build agent (I have not started to check this ... but the escaping thing looks like a possible target)

          Daniel Beck added a comment -

          Probably an issue with the Windows command line functions in core, so reassigning.

          Reducing priority as the obvious workaround is not using text parameters.

          Daniel Beck added a comment - Probably an issue with the Windows command line functions in core, so reassigning. Reducing priority as the obvious workaround is not using text parameters.

          Thomas Wabner added a comment -

          one additional note about the maven project:

          If the job is a maven project and you use such text parameter, also all pre- and post steps will fail (if they invoke for example a maven or ant tool). In such case you can "unset" the parameter with the env-inject plugin and remember it into another variable ... this variable will then not added as a parameter to the job ... but this is still very hackish.

          So the problem exists also for maven jobs with pre- or post-steps.

          I do not know, if it is a good idea to reduce the priority, because of the possibility to "destroy" the Jenkins slave with a unchecked parameter ... but I do not know enough about the JIRA rules for this project.

          Possible more investigation about the consequences should be made, to decide about the priority.

          Thomas Wabner added a comment - one additional note about the maven project: If the job is a maven project and you use such text parameter, also all pre- and post steps will fail (if they invoke for example a maven or ant tool). In such case you can "unset" the parameter with the env-inject plugin and remember it into another variable ... this variable will then not added as a parameter to the job ... but this is still very hackish. So the problem exists also for maven jobs with pre- or post-steps. I do not know, if it is a good idea to reduce the priority, because of the possibility to "destroy" the Jenkins slave with a unchecked parameter ... but I do not know enough about the JIRA rules for this project. Possible more investigation about the consequences should be made, to decide about the priority.

          Daniel Beck added a comment -

          You make a good point. Not enough to be considered a security issue IMO, but can still have very weird consequences.

          Daniel Beck added a comment - You make a good point. Not enough to be considered a security issue IMO, but can still have very weird consequences.

          After a great deal of googling around I am starting to think it is not possible to pass newline in an argument of cmd.exe. I would love to be proven wrong.

          Oliver Gondža added a comment - After a great deal of googling around I am starting to think it is not possible to pass newline in an argument of cmd.exe . I would love to be proven wrong.

          James Nord added a comment - - edited

          why are we even parsing these as system properties to the tool. They should be environment variables which "should then just work (tm)".

          in maven ${env.foobar} rather than ${foobar}

          I'm pretty sure (but not willing to bet my house) that windows supports multiline environment variables.

          James Nord added a comment - - edited why are we even parsing these as system properties to the tool. They should be environment variables which "should then just work (tm)". in maven ${env.foobar } rather than ${foobar } I'm pretty sure (but not willing to bet my house) that windows supports multiline environment variables.

          Daniel Beck added a comment -

          I think the caret is the end of line escape char there.

          Seriously though would this be a breaking change we could do for 2.0? I've never understood the reason behind having parameters passed like this. Sure it's nice sometimes, but overall more trouble that it's worth.

          Daniel Beck added a comment - I think the caret is the end of line escape char there. Seriously though would this be a breaking change we could do for 2.0? I've never understood the reason behind having parameters passed like this. Sure it's nice sometimes, but overall more trouble that it's worth.

          Oliver Gondža added a comment - Fix proposed https://github.com/jenkinsci/jenkins/pull/1976 .

          Daniel Beck added a comment -

          Fix released in 2.12

          Daniel Beck added a comment - Fix released in 2.12

            Unassigned Unassigned
            waffel Thomas Wabner
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: