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

text parameter with newlines causing the default goal run

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: core
    • Labels:
    • Environment:
      Jenkins 1.583, Windows, Java 7U72
    • Similar Issues:

      Description

      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

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

            Is that a freestyle project with maven build step, or a maven project?

            Show
            danielbeck Daniel Beck added a comment - Is that a freestyle project with maven build step, or a maven project?
            Hide
            waffel Thomas Wabner added a comment -

            It is a freestyle project. I have started to check with a maven project ... here the problem seems to not exists.

            I guess it is not maven relevant, because the freestyle project calls mvn.bat ... same problem can be exists for ANT or other tools which are called via cmd.

            Possible it is a general "shell" problem under Windows.

            Show
            waffel Thomas Wabner added a comment - It is a freestyle project. I have started to check with a maven project ... here the problem seems to not exists. I guess it is not maven relevant, because the freestyle project calls mvn.bat ... same problem can be exists for ANT or other tools which are called via cmd. Possible it is a general "shell" problem under Windows.
            Hide
            danielbeck Daniel Beck added a comment -

            Possibly. Could you try with ant as well?

            Show
            danielbeck Daniel Beck added a comment - Possibly. Could you try with ant as well?
            Hide
            waffel 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)

            Show
            waffel 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)
            Hide
            danielbeck 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.

            Show
            danielbeck 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.
            Hide
            waffel 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.

            Show
            waffel 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.
            Hide
            danielbeck 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.

            Show
            danielbeck 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.
            Hide
            olivergondza 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.

            Show
            olivergondza 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.
            Hide
            teilo 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.

            Show
            teilo 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.
            Hide
            danielbeck 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.

            Show
            danielbeck 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.
            Hide
            olivergondza Oliver Gondža added a comment -
            Show
            olivergondza Oliver Gondža added a comment - Fix proposed https://github.com/jenkinsci/jenkins/pull/1976 .
            Hide
            danielbeck Daniel Beck added a comment -

            Fix released in 2.12

            Show
            danielbeck Daniel Beck added a comment - Fix released in 2.12

              People

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

                Dates

                Created:
                Updated:
                Resolved: