CVE-2017-1000487

@@ -139,6 +139,8 @@ public class Commandline
      * 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 )
@@ -167,15 +169,16 @@ 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
      */
@@ -203,7 +206,6 @@ public Commandline( String toProcess )
 
     /**
      * Create a new command line object.
-     * Shell is autodetected from operating system
      */
     public Commandline()
     {
@@ -253,7 +255,7 @@ 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 );
@@ -404,6 +406,21 @@ public void setExecutable( String 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();
@@ -483,7 +500,7 @@ public String[] getEnvironmentVariables()
     public String[] getCommandline()
     {
         final String[] args = getArguments();
-        String executable = getExecutable();
+        String executable = getLiteralExecutable();
 
         if ( executable == null )
         {
@@ -497,6 +514,8 @@ public String[] getCommandline()
 
     /**
      * Returns the shell, executable and all defined arguments.
+     *
+     * Shell usage is only desirable when generating code for remote execution.
      */
     public String[] getShellCommandline()
     {
@@ -633,7 +652,7 @@ public Process execute()
         {
             if ( workingDir == null )
             {
-                process = Runtime.getRuntime().exec( getShellCommandline(), environment );
+                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
             }
             else
             {
@@ -648,7 +667,7 @@ else if ( !workingDir.isDirectory() )
                         + "\" does not specify a directory." );
                 }
 
-                process = Runtime.getRuntime().exec( getShellCommandline(), environment, workingDir );
+                process = Runtime.getRuntime().exec( getCommandline(), environment, workingDir );
             }
         }
         catch ( IOException ex )
@@ -669,7 +688,7 @@ private void verifyShellState()
             shell.setWorkingDirectory( workingDir );
         }
 
-        if ( shell.getExecutable() == null )
+        if ( shell.getOriginalExecutable() == null )
         {
             shell.setExecutable( executable );
         }
@@ -684,6 +703,8 @@ 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
      */
@@ -695,6 +716,7 @@ 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()

@@ -17,7 +17,6 @@
  */
 
 import org.codehaus.plexus.util.Os;
-import org.codehaus.plexus.util.StringUtils;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -29,34 +28,18 @@
 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 );
@@ -76,7 +59,7 @@ public String getExecutable()
             return super.getExecutable();
         }
 
-        return unifyQuotes( super.getExecutable());
+        return quoteOneItem( super.getOriginalExecutable(), true );
     }
 
     public List<String> getShellArgsList()
@@ -126,46 +109,41 @@ protected String getExecutionPreamble()
         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();
     }
 }

@@ -48,6 +48,8 @@ public class Shell
 
     private boolean quotedArgumentsEnabled = true;
 
+    private boolean unconditionallyQuote = false;
+
     private String executable;
 
     private String workingDir;
@@ -68,6 +70,16 @@ public class Shell
 
     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,...)
      *
@@ -129,6 +141,19 @@ 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>();
@@ -144,9 +169,7 @@ protected List<String> getRawCommandLine( String executable, String[] arguments
 
             if ( isQuotedExecutableEnabled() )
             {
-                char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
-
-                sb.append( StringUtils.quoteAndEscape( getExecutable(), getExecutableQuoteDelimiter(), escapeChars, getQuotingTriggerChars(), '\\', false ) );
+                sb.append( quoteOneItem( getOriginalExecutable(), true ) );
             }
             else
             {
@@ -162,9 +185,7 @@ protected List<String> getRawCommandLine( String executable, String[] arguments
 
             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
             {
@@ -278,7 +299,7 @@ public List<String> getShellCommandLine( String[] arguments )
             commandLine.addAll( getShellArgsList() );
         }
 
-        commandLine.addAll( getCommandLine( getExecutable(), arguments ) );
+        commandLine.addAll( getCommandLine( getOriginalExecutable(), arguments ) );
 
         return commandLine;
 

@@ -16,22 +16,15 @@
  * 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
@@ -252,7 +245,7 @@ public void testGetShellCommandLineBash()
 
         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\'";
@@ -282,12 +275,12 @@ public void testGetShellCommandLineBash_WithWorkingDirectory()
 
         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] );
     }
@@ -311,7 +304,7 @@ public void testGetShellCommandLineBash_WithSingleQuotedArg()
 
         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\'";
@@ -341,7 +334,7 @@ public void testGetShellCommandLineNonWindows()
         }
         else
         {
-            assertEquals( "/usr/bin a b", shellCommandline[2] );
+            assertEquals( "'/usr/bin' 'a' 'b'", shellCommandline[2] );
         }
     }
 
@@ -387,6 +380,18 @@ public void testQuotedPathWithSingleApostrophe()
         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.
      *

@@ -16,14 +16,13 @@
  * 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
 {
@@ -42,7 +41,7 @@ public void testQuoteWorkingDirectoryAndExecutable()
 
         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()
@@ -54,7 +53,7 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes()
 
         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()
@@ -66,7 +65,7 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_Backsl
 
         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()
@@ -82,7 +81,7 @@ public void testPreserveSingleQuotesOnArgument()
 
         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()
@@ -114,7 +113,7 @@ public void testEscapeSingleQuotesOnArgument()
 
         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()
@@ -146,7 +145,7 @@ public void testArgumentsWithsemicolon()
 
         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" );
@@ -158,7 +157,7 @@ public void testArgumentsWithsemicolon()
 
         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 );
@@ -206,7 +205,7 @@ public void testBourneShellQuotingCharacters()
 
         assertEquals( "/bin/sh", lines[0] );
         assertEquals( "-c", lines[1] );
-        assertEquals( "chmod ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
+        assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
                       lines[2] );
 
     }