CVE-2017-1000487: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: Plexus-utils before 3.0.16 is vulnerable to command injection because it does not correctly process the contents of double quoted strings.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2018-01-03T20:29Z
Repository: https://github.com/codehaus-plexus/plexus-utils
Language: Java
Files changed: src/main/java/org/codehaus/plexus/util/cli/Commandline.java, src/main/java/org/codehaus/plexus/util/cli/shell/BourneShell.java, src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java, src/test/java/org/codehaus/plexus/util/cli/CommandlineTest.java, src/test/java/org/codehaus/plexus/util/cli/shell/BourneShellTest.java
Commit date: 2013-10-23 19:48:13 +0200
Commit message: [PLXUTILS-161] Commandline shell injection problems

Patch by Charles Duffy, applied unmodified

Diff: diff --git a/src/main/java/org/codehaus/plexus/util/cli/Commandline.java b/src/main/java/org/codehaus/plexus/util/cli/Commandline.java
index 5e0d5af..7346c7e 100644
--- a/src/main/java/org/codehaus/plexus/util/cli/Commandline.java
+++ b/src/main/java/org/codehaus/plexus/util/cli/Commandline.java
@@ -137,10 +137,12 @@
 
     /**
      * Create a new command line object.
      * Shell is autodetected from operating system
      *
+     * Shell usage is only desirable when generating code for remote execution.
+     *
      * @param toProcess
      */
     public Commandline( String toProcess, Shell shell )
     {
         this.shell = shell;
@@ -165,19 +167,20 @@ public Commandline( String toProcess, Shell shell )
     }
 
     /**
      * Create a new command line object.
      * Shell is autodetected from operating system
+     *
+     * Shell usage is only desirable when generating code for remote execution.
      */
     public Commandline( Shell shell )
     {
         this.shell = shell;
     }
 
     /**
-     * Create a new command line object.
-     * Shell is autodetected from operating system
+     * Create a new command line object, given a command following POSIX sh quoting rules
      *
      * @param toProcess
      */
     public Commandline( String toProcess )
     {
@@ -201,11 +204,10 @@ public Commandline( String toProcess )
         }
     }
 
     /**
      * Create a new command line object.
-     * Shell is autodetected from operating system
      */
     public Commandline()
     {
         setDefaultShell();
     }
@@ -251,11 +253,11 @@ public void setPid( long pid )
          */
         public int getPosition()
         {
             if ( realPos == -1 )
             {
-                realPos = ( getExecutable() == null ? 0 : 1 );
+                realPos = ( getLiteralExecutable() == null ? 0 : 1 );
                 for ( int i = 0; i < position; i++ )
                 {
                     Arg arg = (Arg) arguments.elementAt( i );
                     realPos += arg.getParts().length;
                 }
@@ -402,10 +404,25 @@ public void setExecutable( String executable )
     {
         shell.setExecutable( executable );
         this.executable = executable;
     }
 
+    /**
+     * @return Executable to be run, as a literal string (no shell quoting/munging)
+     */
+    public String getLiteralExecutable()
+    {
+        return executable;
+    }
+
+    /**
+     * Return an executable name, quoted for shell use.
+     *
+     * Shell usage is only desirable when generating code for remote execution.
+     *
+     * @return Executable to be run, quoted for shell interpretation
+     */
     public String getExecutable()
     {
         String exec = shell.getExecutable();
 
         if ( exec == null )
@@ -481,11 +498,11 @@ public String[] getEnvironmentVariables()
      * Returns the executable and all defined arguments.
      */
     public String[] getCommandline()
     {
         final String[] args = getArguments();
-        String executable = getExecutable();
+        String executable = getLiteralExecutable();
 
         if ( executable == null )
         {
             return args;
         }
@@ -495,10 +512,12 @@ public String[] getCommandline()
         return result;
     }
 
     /**
      * Returns the shell, executable and all defined arguments.
+     *
+     * Shell usage is only desirable when generating code for remote execution.
      */
     public String[] getShellCommandline()
     {
         // TODO: Provided only for backward compat. with <= 1.4
         verifyShellState();
@@ -631,11 +650,11 @@ public Process execute()
 
         try
         {
             if ( workingDir == null )
             {
-                process = Runtime.getRuntime().exec( getShellCommandline(), environment );
+                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
             }
             else
             {
                 if ( !workingDir.exists() )
                 {
@@ -646,11 +665,11 @@ else if ( !workingDir.isDirectory() )
                 {
                     throw new CommandLineException( "Path \"" + workingDir.getPath()
                         + "\" does not specify a directory." );
                 }
 
-                process = Runtime.getRuntime().exec( getShellCommandline(), environment, workingDir );
+                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
             }
         }
         catch ( IOException ex )
         {
             throw new CommandLineException( "Error while executing process.", ex );
@@ -667,11 +686,11 @@ private void verifyShellState()
         if ( shell.getWorkingDirectory() == null )
         {
             shell.setWorkingDirectory( workingDir );
         }
 
-        if ( shell.getExecutable() == null )
+        if ( shell.getOriginalExecutable() == null )
         {
             shell.setExecutable( executable );
         }
     }
 
@@ -682,10 +701,12 @@ public Properties getSystemEnvVars()
     }
 
     /**
      * Allows to set the shell to be used in this command line.
      *
+     * Shell usage is only desirable when generating code for remote execution.
+     *
      * @param shell
      * @since 1.2
      */
     public void setShell( Shell shell )
     {
@@ -693,10 +714,11 @@ public void setShell( Shell shell )
     }
 
     /**
      * Get the shell to be used in this command line.
      *
+     * Shell usage is only desirable when generating code for remote execution.
      * @since 1.2
      */
     public Shell getShell()
     {
         return shell;
diff --git a/src/main/java/org/codehaus/plexus/util/cli/shell/BourneShell.java b/src/main/java/org/codehaus/plexus/util/cli/shell/BourneShell.java
index d343282..9bf3a09 100644
--- a/src/main/java/org/codehaus/plexus/util/cli/shell/BourneShell.java
+++ b/src/main/java/org/codehaus/plexus/util/cli/shell/BourneShell.java
@@ -15,11 +15,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 import org.codehaus.plexus.util.Os;
-import org.codehaus.plexus.util.StringUtils;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -27,38 +26,22 @@
  * @version $Id$
  */
 public class BourneShell
     extends Shell
 {
-    private static final char[] BASH_QUOTING_TRIGGER_CHARS = {
-        ' ',
-        '$',
-        ';',
-        '&',
-        '|',
-        '<',
-        '>',
-        '*',
-        '?',
-        '(',
-        ')',
-        '[',
-        ']',
-        '{',
-        '}',
-        '`' };
 
     public BourneShell()
     {
-        this( false );
+        this(false);
     }
 
     public BourneShell( boolean isLoginShell )
     {
+        setUnconditionalQuoting( true );
         setShellCommand( "/bin/sh" );
         setArgumentQuoteDelimiter( '\'' );
-        setExecutableQuoteDelimiter( '\"' );
+        setExecutableQuoteDelimiter( '\'' );
         setSingleQuotedArgumentEscaped( true );
         setSingleQuotedExecutableEscaped( false );
         setQuotedExecutableEnabled( true );
         setArgumentEscapePattern("'\\%s'");
 
@@ -74,11 +57,11 @@ public String getExecutable()
         if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
         {
             return super.getExecutable();
         }
 
-        return unifyQuotes( super.getExecutable());
+        return quoteOneItem( super.getOriginalExecutable(), true );
     }
 
     public List<String> getShellArgsList()
     {
         List<String> shellArgs = new ArrayList<String>();
@@ -124,48 +107,43 @@ protected String getExecutionPreamble()
 
         String dir = getWorkingDirectoryAsString();
         StringBuilder sb = new StringBuilder();
         sb.append( "cd " );
 
-        sb.append( unifyQuotes( dir ) );
+        sb.append( quoteOneItem( dir, false ) );
         sb.append( " && " );
 
         return sb.toString();
     }
 
-    protected char[] getQuotingTriggerChars()
-    {
-        return BASH_QUOTING_TRIGGER_CHARS;
-    }
-
     /**
      * <p>Unify quotes in a path for the Bourne Shell.</p>
      *
      * <pre>
-     * BourneShell.unifyQuotes(null)                       = null
-     * BourneShell.unifyQuotes("")                         = (empty)
-     * BourneShell.unifyQuotes("/test/quotedpath'abc")     = /test/quotedpath\'abc
-     * BourneShell.unifyQuotes("/test/quoted path'abc")    = "/test/quoted path'abc"
-     * BourneShell.unifyQuotes("/test/quotedpath\"abc")    = "/test/quotedpath\"abc"
-     * BourneShell.unifyQuotes("/test/quoted path\"abc")   = "/test/quoted path\"abc"
-     * BourneShell.unifyQuotes("/test/quotedpath\"'abc")   = "/test/quotedpath\"'abc"
-     * BourneShell.unifyQuotes("/test/quoted path\"'abc")  = "/test/quoted path\"'abc"
+     * BourneShell.quoteOneItem(null)                       = null
+     * BourneShell.quoteOneItem("")                         = ''
+     * BourneShell.quoteOneItem("/test/quotedpath'abc")     = '/test/quotedpath'"'"'abc'
+     * BourneShell.quoteOneItem("/test/quoted path'abc")    = '/test/quoted pat'"'"'habc'
+     * BourneShell.quoteOneItem("/test/quotedpath\"abc")    = '/test/quotedpath"abc'
+     * BourneShell.quoteOneItem("/test/quoted path\"abc")   = '/test/quoted path"abc'
+     * BourneShell.quoteOneItem("/test/quotedpath\"'abc")   = '/test/quotedpath"'"'"'abc'
+     * BourneShell.quoteOneItem("/test/quoted path\"'abc")  = '/test/quoted path"'"'"'abc'
      * </pre>
      *
      * @param path not null path.
      * @return the path unified correctly for the Bourne shell.
      */
-    protected static String unifyQuotes( String path )
+    protected String quoteOneItem( String path, boolean isExecutable )
     {
         if ( path == null )
         {
             return null;
         }
 
-        if ( path.indexOf( " " ) == -1 && path.indexOf( "'" ) != -1 && path.indexOf( "\"" ) == -1 )
-        {
-            return StringUtils.escape( path );
-        }
+        StringBuilder sb = new StringBuilder();
+        sb.append( "'" );
+        sb.append( path.replace( "'", "'\"'\"'" ) );
+        sb.append( "'" );
 
-        return StringUtils.quoteAndEscape( path, '\"', BASH_QUOTING_TRIGGER_CHARS );
+        return sb.toString();
     }
 }
diff --git a/src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java b/src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java
index 6d748fb..14fd62d 100644
--- a/src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java
+++ b/src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java
@@ -46,10 +46,12 @@
 
     private List<String> shellArgs = new ArrayList<String>();
 
     private boolean quotedArgumentsEnabled = true;
 
+    private boolean unconditionallyQuote = false;
+
     private String executable;
 
     private String workingDir;
 
     private boolean quotedExecutableEnabled = true;
@@ -66,10 +68,20 @@
 
     private char exeQuoteDelimiter = '\"';
 
     private String argumentEscapePattern = "\\%s";
 
+    /**
+     * Toggle unconditional quoting
+     *
+     * @param unconditionallyQuote
+     */
+    public void setUnconditionalQuoting(boolean unconditionallyQuote)
+    {
+        this.unconditionallyQuote = unconditionallyQuote;
+    }
+
     /**
      * Set the command to execute the shell (eg. COMMAND.COM, /bin/bash,...)
      *
      * @param shellCommand
      */
@@ -127,10 +139,23 @@ public String[] getShellArgs()
     public List<String> getCommandLine( String executable, String[] arguments )
     {
         return getRawCommandLine( executable, arguments );
     }
 
+    protected String quoteOneItem(String inputString, boolean isExecutable)
+    {
+        char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
+        return StringUtils.quoteAndEscape(
+            inputString,
+            isExecutable ? getExecutableQuoteDelimiter() : getArgumentQuoteDelimiter(),
+            escapeChars,
+            getQuotingTriggerChars(),
+            '\\',
+            unconditionallyQuote
+        );
+    }
+
     protected List<String> getRawCommandLine( String executable, String[] arguments )
     {
         List<String> commandLine = new ArrayList<String>();
         StringBuilder sb = new StringBuilder();
 
@@ -142,13 +167,11 @@ protected List<String> getRawCommandLine( String executable, String[] arguments
                 sb.append( preamble );
             }
 
             if ( isQuotedExecutableEnabled() )
             {
-                char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
-
-                sb.append( StringUtils.quoteAndEscape( getExecutable(), getExecutableQuoteDelimiter(), escapeChars, getQuotingTriggerChars(), '\\', false ) );
+                sb.append( quoteOneItem( getOriginalExecutable(), true ) );
             }
             else
             {
                 sb.append( getExecutable() );
             }
@@ -160,13 +183,11 @@ protected List<String> getRawCommandLine( String executable, String[] arguments
                 sb.append( " " );
             }
 
             if ( isQuotedArgumentsEnabled() )
             {
-                char[] escapeChars = getEscapeChars( isSingleQuotedArgumentEscaped(), isDoubleQuotedArgumentEscaped() );
-
-                sb.append( StringUtils.quoteAndEscape( arguments[i], getArgumentQuoteDelimiter(), escapeChars, getQuotingTriggerChars(), getArgumentEscapePattern(), false ) );
+                sb.append( quoteOneItem( arguments[i], false ) );
             }
             else
             {
                 sb.append( arguments[i] );
             }
@@ -276,11 +297,11 @@ public List<String> getShellCommandLine( String[] arguments )
         if ( getShellArgs() != null )
         {
             commandLine.addAll( getShellArgsList() );
         }
 
-        commandLine.addAll( getCommandLine( getExecutable(), arguments ) );
+        commandLine.addAll( getCommandLine( getOriginalExecutable(), arguments ) );
 
         return commandLine;
 
     }
 
diff --git a/src/test/java/org/codehaus/plexus/util/cli/CommandlineTest.java b/src/test/java/org/codehaus/plexus/util/cli/CommandlineTest.java
index 2dac7d9..7d9e3df 100644
--- a/src/test/java/org/codehaus/plexus/util/cli/CommandlineTest.java
+++ b/src/test/java/org/codehaus/plexus/util/cli/CommandlineTest.java
@@ -14,26 +14,19 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
+import junit.framework.TestCase;
 import org.codehaus.plexus.util.IOUtil;
 import org.codehaus.plexus.util.Os;
 import org.codehaus.plexus.util.StringUtils;
 import org.codehaus.plexus.util.cli.shell.BourneShell;
 import org.codehaus.plexus.util.cli.shell.CmdShell;
 import org.codehaus.plexus.util.cli.shell.Shell;
 
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.Reader;
-import java.io.StringWriter;
-import java.io.Writer;
-
-import junit.framework.TestCase;
+import java.io.*;
 
 public class CommandlineTest
     extends TestCase
 {
     private String baseDir;
@@ -250,11 +243,11 @@ public void testGetShellCommandLineBash()
 
         assertEquals( "Command line size", 3, shellCommandline.length );
 
         assertEquals( "/bin/sh", shellCommandline[0] );
         assertEquals( "-c", shellCommandline[1] );
-        String expectedShellCmd = "/bin/echo \'hello world\'";
+        String expectedShellCmd = "'/bin/echo' 'hello world'";
         if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
         {
             expectedShellCmd = "\\bin\\echo \'hello world\'";
         }
         assertEquals( expectedShellCmd, shellCommandline[2] );
@@ -280,16 +273,16 @@ public void testGetShellCommandLineBash_WithWorkingDirectory()
 
         assertEquals( "Command line size", 3, shellCommandline.length );
 
         assertEquals( "/bin/sh", shellCommandline[0] );
         assertEquals( "-c", shellCommandline[1] );
-        String expectedShellCmd = "cd \"" + root.getAbsolutePath()
-                                  + "path with spaces\" && /bin/echo \'hello world\'";
+        String expectedShellCmd = "cd '" + root.getAbsolutePath()
+                                  + "path with spaces' && '/bin/echo' 'hello world'";
         if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
         {
-            expectedShellCmd = "cd \"" + root.getAbsolutePath()
-                               + "path with spaces\" && \\bin\\echo \'hello world\'";
+            expectedShellCmd = "cd '" + root.getAbsolutePath()
+                               + "path with spaces' && '\\bin\\echo' 'hello world'";
         }
         assertEquals( expectedShellCmd, shellCommandline[2] );
     }
 
     /**
@@ -309,11 +302,11 @@ public void testGetShellCommandLineBash_WithSingleQuotedArg()
 
         assertEquals( "Command line size", 3, shellCommandline.length );
 
         assertEquals( "/bin/sh", shellCommandline[0] );
         assertEquals( "-c", shellCommandline[1] );
-        String expectedShellCmd = "/bin/echo \'hello world\'";
+        String expectedShellCmd = "'/bin/echo' ''\"'\"'hello world'\"'\"''";
         if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
         {
             expectedShellCmd = "\\bin\\echo \'hello world\'";
         }
         assertEquals( expectedShellCmd, shellCommandline[2] );
@@ -339,11 +332,11 @@ public void testGetShellCommandLineNonWindows()
         {
             assertEquals( "\\usr\\bin a b", shellCommandline[2] );
         }
         else
         {
-            assertEquals( "/usr/bin a b", shellCommandline[2] );
+            assertEquals( "'/usr/bin' 'a' 'b'", shellCommandline[2] );
         }
     }
 
     public void testEnvironment()
         throws Exception
@@ -385,10 +378,22 @@ public void testQuotedPathWithSingleApostrophe()
 
         dir = new File( System.getProperty( "basedir" ), "target/test/quoted path'test" );
         createAndCallScript( dir, "echo Quoted" );
     }
 
+    /**
+     * Test an executable with shell-expandable content in its path.
+     *
+     * @throws Exception
+     */
+    public void testPathWithShellExpansionStrings()
+        throws Exception
+    {
+        File dir = new File( System.getProperty( "basedir" ), "target/test/dollar$test" );
+        createAndCallScript( dir, "echo Quoted" );
+    }
+
     /**
      * Test an executable with a single quotation mark <code>\"</code> in its path only for non Windows box.
      *
      * @throws Exception
      */
diff --git a/src/test/java/org/codehaus/plexus/util/cli/shell/BourneShellTest.java b/src/test/java/org/codehaus/plexus/util/cli/shell/BourneShellTest.java
index 2a987ed..0e06c63 100644
--- a/src/test/java/org/codehaus/plexus/util/cli/shell/BourneShellTest.java
+++ b/src/test/java/org/codehaus/plexus/util/cli/shell/BourneShellTest.java
@@ -14,18 +14,17 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
+import junit.framework.TestCase;
 import org.codehaus.plexus.util.StringUtils;
 import org.codehaus.plexus.util.cli.Commandline;
 
 import java.util.Arrays;
 import java.util.List;
 
-import junit.framework.TestCase;
-
 public class BourneShellTest
     extends TestCase
 {
 
     protected Shell newShell()
@@ -40,11 +39,11 @@ public void testQuoteWorkingDirectoryAndExecutable()
         sh.setWorkingDirectory( "/usr/local/bin" );
         sh.setExecutable( "chmod" );
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd /usr/local/bin && chmod", executable );
+        assertEquals( "/bin/sh -c cd '/usr/local/bin' && 'chmod'", executable );
     }
 
     public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes()
     {
         Shell sh = newShell();
@@ -52,11 +51,11 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes()
         sh.setWorkingDirectory( "/usr/local/'something else'" );
         sh.setExecutable( "chmod" );
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd \"/usr/local/\'something else\'\" && chmod", executable );
+        assertEquals( "/bin/sh -c cd '/usr/local/'\"'\"'something else'\"'\"'' && 'chmod'", executable );
     }
 
     public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_BackslashFileSep()
     {
         Shell sh = newShell();
@@ -64,11 +63,11 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_Backsl
         sh.setWorkingDirectory( "\\usr\\local\\'something else'" );
         sh.setExecutable( "chmod" );
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd \"\\usr\\local\\\'something else\'\" && chmod", executable );
+        assertEquals( "/bin/sh -c cd '\\usr\\local\\\'\"'\"'something else'\"'\"'' && 'chmod'", executable );
     }
 
     public void testPreserveSingleQuotesOnArgument()
     {
         Shell sh = newShell();
@@ -80,11 +79,11 @@ public void testPreserveSingleQuotesOnArgument()
 
         List shellCommandLine = sh.getShellCommandLine( args );
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( args[0] ) );
+        assertTrue( cli.endsWith("''\"'\"'some arg with spaces'\"'\"''"));
     }
 
     public void testAddSingleQuotesOnArgumentWithSpaces()
     {
         Shell sh = newShell();
@@ -112,11 +111,11 @@ public void testEscapeSingleQuotesOnArgument()
 
         List shellCommandLine = sh.getShellCommandLine( args );
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertEquals("cd /usr/bin && chmod 'arg'\\''withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
+        assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
     }
 
     public void testArgumentsWithsemicolon()
     {
 
@@ -144,11 +143,11 @@ public void testArgumentsWithsemicolon()
         String[] lines = commandline.getShellCommandline();
         System.out.println( Arrays.asList( lines ) );
 
         assertEquals( "/bin/sh", lines[0] );
         assertEquals( "-c", lines[1] );
-        assertEquals( "chmod --password ';password'", lines[2] );
+        assertEquals( "'chmod' '--password' ';password'", lines[2] );
 
         commandline = new Commandline( newShell() );
         commandline.setExecutable( "chmod" );
         commandline.getShell().setQuotedArgumentsEnabled( true );
         commandline.createArg().setValue( "--password" );
@@ -156,11 +155,11 @@ public void testArgumentsWithsemicolon()
         lines = commandline.getShellCommandline();
         System.out.println( Arrays.asList( lines ) );
 
         assertEquals( "/bin/sh", lines[0] );
         assertEquals( "-c", lines[1] );
-        assertEquals( "chmod --password ';password'", lines[2] );
+        assertEquals( "'chmod' '--password' ';password'", lines[2] );
 
         commandline = new Commandline( new CmdShell() );
         commandline.getShell().setQuotedArgumentsEnabled( true );
         commandline.createArg().setValue( "--password" );
         commandline.createArg().setValue( ";password" );
@@ -204,11 +203,11 @@ public void testBourneShellQuotingCharacters()
         String[] lines = commandline.getShellCommandline();
         System.out.println( Arrays.asList( lines ) );
 
         assertEquals( "/bin/sh", lines[0] );
         assertEquals( "-c", lines[1] );
-        assertEquals( "chmod ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
+        assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
                       lines[2] );
 
     }
 
 }

Vulnerable code:      * Create a new command line object.
     * Shell is autodetected from operating system
     * Shell is autodetected from operating system
                realPos = ( getExecutable() == null ? 0 : 1 );
        String executable = getExecutable();
                process = Runtime.getRuntime().exec( getShellCommandline(), environment );
                process = Runtime.getRuntime().exec( getShellCommandline(), environment, workingDir );
        if ( shell.getExecutable() == null )
import org.codehaus.plexus.util.StringUtils;
    private static final char[] BASH_QUOTING_TRIGGER_CHARS = {
        ' ',
        '$',
        ';',
        '&',
        '|',
        '<',
        '>',
        '*',
        '?',
        '(',
        ')',
        '[',
        ']',
        '{',
        '}',
        '`' };
        this( false );
        setExecutableQuoteDelimiter( '\"' );
        return unifyQuotes( super.getExecutable());
        sb.append( unifyQuotes( dir ) );
    protected char[] getQuotingTriggerChars()
    {
        return BASH_QUOTING_TRIGGER_CHARS;
    }

     * BourneShell.unifyQuotes(null)                       = null
     * BourneShell.unifyQuotes("")                         = (empty)
     * BourneShell.unifyQuotes("/test/quotedpath'abc")     = /test/quotedpath\'abc
     * BourneShell.unifyQuotes("/test/quoted path'abc")    = "/test/quoted path'abc"
     * BourneShell.unifyQuotes("/test/quotedpath\"abc")    = "/test/quotedpath\"abc"
     * BourneShell.unifyQuotes("/test/quoted path\"abc")   = "/test/quoted path\"abc"
     * BourneShell.unifyQuotes("/test/quotedpath\"'abc")   = "/test/quotedpath\"'abc"
     * BourneShell.unifyQuotes("/test/quoted path\"'abc")  = "/test/quoted path\"'abc"
    protected static String unifyQuotes( String path )
        if ( path.indexOf( " " ) == -1 && path.indexOf( "'" ) != -1 && path.indexOf( "\"" ) == -1 )
        {
            return StringUtils.escape( path );
        }
        return StringUtils.quoteAndEscape( path, '\"', BASH_QUOTING_TRIGGER_CHARS );
                char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );

                sb.append( StringUtils.quoteAndEscape( getExecutable(), getExecutableQuoteDelimiter(), escapeChars, getQuotingTriggerChars(), '\\', false ) );
                char[] escapeChars = getEscapeChars( isSingleQuotedArgumentEscaped(), isDoubleQuotedArgumentEscaped() );

                sb.append( StringUtils.quoteAndEscape( arguments[i], getArgumentQuoteDelimiter(), escapeChars, getQuotingTriggerChars(), getArgumentEscapePattern(), false ) );
        commandLine.addAll( getCommandLine( getExecutable(), arguments ) );
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringWriter;
import java.io.Writer;

import junit.framework.TestCase;
        String expectedShellCmd = "/bin/echo \'hello world\'";
        String expectedShellCmd = "cd \"" + root.getAbsolutePath()
                                  + "path with spaces\" && /bin/echo \'hello world\'";
            expectedShellCmd = "cd \"" + root.getAbsolutePath()
                               + "path with spaces\" && \\bin\\echo \'hello world\'";
        String expectedShellCmd = "/bin/echo \'hello world\'";
            assertEquals( "/usr/bin a b", shellCommandline[2] );
import junit.framework.TestCase;

        assertEquals( "/bin/sh -c cd /usr/local/bin && chmod", executable );
        assertEquals( "/bin/sh -c cd \"/usr/local/\'something else\'\" && chmod", executable );
        assertEquals( "/bin/sh -c cd \"\\usr\\local\\\'something else\'\" && chmod", executable );
        assertTrue( cli.endsWith( args[0] ) );
        assertEquals("cd /usr/bin && chmod 'arg'\\''withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
        assertEquals( "chmod --password ';password'", lines[2] );
        assertEquals( "chmod --password ';password'", lines[2] );
        assertEquals( "chmod ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
Fixed code:      * Shell usage is only desirable when generating code for remote execution.
     *
     *
     * Shell usage is only desirable when generating code for remote execution.
     * Create a new command line object, given a command following POSIX sh quoting rules
                realPos = ( getLiteralExecutable() == null ? 0 : 1 );
    /**
     * @return Executable to be run, as a literal string (no shell quoting/munging)
     */
    public String getLiteralExecutable()
    {
        return executable;
    }

    /**
     * Return an executable name, quoted for shell use.
     *
     * Shell usage is only desirable when generating code for remote execution.
     *
     * @return Executable to be run, quoted for shell interpretation
     */
        String executable = getLiteralExecutable();
     *
     * Shell usage is only desirable when generating code for remote execution.
                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
        if ( shell.getOriginalExecutable() == null )
     * Shell usage is only desirable when generating code for remote execution.
     *
     * Shell usage is only desirable when generating code for remote execution.
        this(false);
        setUnconditionalQuoting( true );
        setExecutableQuoteDelimiter( '\'' );
        return quoteOneItem( super.getOriginalExecutable(), true );
        sb.append( quoteOneItem( dir, false ) );
     * BourneShell.quoteOneItem(null)                       = null
     * BourneShell.quoteOneItem("")                         = ''
     * BourneShell.quoteOneItem("/test/quotedpath'abc")     = '/test/quotedpath'"'"'abc'
     * BourneShell.quoteOneItem("/test/quoted path'abc")    = '/test/quoted pat'"'"'habc'
     * BourneShell.quoteOneItem("/test/quotedpath\"abc")    = '/test/quotedpath"abc'
     * BourneShell.quoteOneItem("/test/quoted path\"abc")   = '/test/quoted path"abc'
     * BourneShell.quoteOneItem("/test/quotedpath\"'abc")   = '/test/quotedpath"'"'"'abc'
     * BourneShell.quoteOneItem("/test/quoted path\"'abc")  = '/test/quoted path"'"'"'abc'
    protected String quoteOneItem( String path, boolean isExecutable )
        StringBuilder sb = new StringBuilder();
        sb.append( "'" );
        sb.append( path.replace( "'", "'\"'\"'" ) );
        sb.append( "'" );
        return sb.toString();
    private boolean unconditionallyQuote = false;

    /**
     * Toggle unconditional quoting
     *
     * @param unconditionallyQuote
     */
    public void setUnconditionalQuoting(boolean unconditionallyQuote)
    {
        this.unconditionallyQuote = unconditionallyQuote;
    }

    protected String quoteOneItem(String inputString, boolean isExecutable)
    {
        char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
        return StringUtils.quoteAndEscape(
            inputString,
            isExecutable ? getExecutableQuoteDelimiter() : getArgumentQuoteDelimiter(),
            escapeChars,
            getQuotingTriggerChars(),
            '\\',
            unconditionallyQuote
        );
    }

                sb.append( quoteOneItem( getOriginalExecutable(), true ) );
                sb.append( quoteOneItem( arguments[i], false ) );
        commandLine.addAll( getCommandLine( getOriginalExecutable(), arguments ) );
import junit.framework.TestCase;
import java.io.*;
        String expectedShellCmd = "'/bin/echo' 'hello world'";
        String expectedShellCmd = "cd '" + root.getAbsolutePath()
                                  + "path with spaces' && '/bin/echo' 'hello world'";
            expectedShellCmd = "cd '" + root.getAbsolutePath()
                               + "path with spaces' && '\\bin\\echo' 'hello world'";
        String expectedShellCmd = "'/bin/echo' ''\"'\"'hello world'\"'\"''";
            assertEquals( "'/usr/bin' 'a' 'b'", shellCommandline[2] );
    /**
     * Test an executable with shell-expandable content in its path.
     *
     * @throws Exception
     */
    public void testPathWithShellExpansionStrings()
        throws Exception
    {
        File dir = new File( System.getProperty( "basedir" ), "target/test/dollar$test" );
        createAndCallScript( dir, "echo Quoted" );
    }

import junit.framework.TestCase;
        assertEquals( "/bin/sh -c cd '/usr/local/bin' && 'chmod'", executable );
        assertEquals( "/bin/sh -c cd '/usr/local/'\"'\"'something else'\"'\"'' && 'chmod'", executable );
        assertEquals( "/bin/sh -c cd '\\usr\\local\\\'\"'\"'something else'\"'\"'' && 'chmod'", executable );
        assertTrue( cli.endsWith("''\"'\"'some arg with spaces'\"'\"''"));
        assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
        assertEquals( "'chmod' '--password' ';password'", lines[2] );
        assertEquals( "'chmod' '--password' ';password'", lines[2] );
        assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",