diff --git a/SOURCES/0001-Unconditionally-single-quote-executable-and-argument.patch b/SOURCES/0001-Unconditionally-single-quote-executable-and-argument.patch new file mode 100644 index 0000000..d20e146 --- /dev/null +++ b/SOURCES/0001-Unconditionally-single-quote-executable-and-argument.patch @@ -0,0 +1,330 @@ +From ad1c8a07230c9e0ba26da54fa383ecefde949cb5 Mon Sep 17 00:00:00 2001 +From: Marian Koncek +Date: Fri, 8 Apr 2022 11:55:26 +0200 +Subject: [PATCH] Unconditionally single quote executable and arguments + +Upstream: https://github.com/apache/maven-shared-utils/pull/40/commits + +--- + .../shared/utils/cli/shell/BourneShell.java | 45 ++++++++----------- + .../maven/shared/utils/cli/shell/Shell.java | 43 ++++++++++++------ + .../utils/cli/shell/BourneShellTest.java | 35 ++++++++++----- + 3 files changed, 73 insertions(+), 50 deletions(-) + +diff --git a/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java b/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java +index 845e4a4..fc4984a 100644 +--- a/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java ++++ b/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java +@@ -23,7 +23,6 @@ package org.apache.maven.shared.utils.cli.shell; + import java.util.ArrayList; + import java.util.List; + import org.apache.maven.shared.utils.Os; +-import org.apache.maven.shared.utils.StringUtils; + + /** + * @author Jason van Zyl +@@ -31,14 +30,12 @@ import org.apache.maven.shared.utils.StringUtils; + public class BourneShell + extends Shell + { +- private static final char[] BASH_QUOTING_TRIGGER_CHARS = +- { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`' }; +- + public BourneShell() + { ++ setUnconditionalQuoting( true ); + setShellCommand( "/bin/sh" ); + setArgumentQuoteDelimiter( '\'' ); +- setExecutableQuoteDelimiter( '\"' ); ++ setExecutableQuoteDelimiter( '\'' ); + setSingleQuotedArgumentEscaped( true ); + setSingleQuotedExecutableEscaped( false ); + setQuotedExecutableEnabled( true ); +@@ -54,7 +51,7 @@ public class BourneShell + return super.getExecutable(); + } + +- return unifyQuotes( super.getExecutable() ); ++ return quoteOneItem( super.getExecutable(), true ); + } + + public List getShellArgsList() +@@ -104,46 +101,40 @@ public class BourneShell + 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; +- } +- + /** + *

Unify quotes in a path for the Bourne Shell.

+ *

+ *

+-     * 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'
+      * 
+ * + * @param path not null path. + * @return the path unified correctly for the Bourne shell. + */ +- private 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 ); +- } +- +- return StringUtils.quoteAndEscape( path, '\"', BASH_QUOTING_TRIGGER_CHARS ); ++ StringBuilder sb = new StringBuilder(); ++ sb.append( "'" ); ++ sb.append( path.replace( "'", "'\"'\"'" ) ); ++ sb.append( "'" ); ++ return sb.toString(); + } + } +diff --git a/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java b/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java +index 70dddd0..2b889ff 100644 +--- a/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java ++++ b/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java +@@ -50,6 +50,8 @@ public class Shell + + private boolean quotedArgumentsEnabled = true; + ++ private boolean unconditionalQuoting = false; ++ + private String executable; + + private String workingDir; +@@ -113,6 +115,19 @@ public class Shell + } + } + ++ protected String quoteOneItem( String inputString, boolean isExecutable ) ++ { ++ char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() ); ++ return StringUtils.quoteAndEscape( ++ inputString, ++ isExecutable ? getExecutableQuoteDelimiter() : getArgumentQuoteDelimiter(), ++ escapeChars, ++ getQuotingTriggerChars(), ++ '\\', ++ unconditionalQuoting ++ ); ++ } ++ + /** + * Get the command line for the provided executable and arguments in this shell + * +@@ -125,12 +140,12 @@ public class Shell + return getRawCommandLine( executable, arguments ); + } + +- List getRawCommandLine( String executable, String... arguments ) ++ List getRawCommandLine( String executableParameter, String... arguments ) + { + List commandLine = new ArrayList(); + StringBuilder sb = new StringBuilder(); + +- if ( executable != null ) ++ if ( executableParameter != null ) + { + String preamble = getExecutionPreamble(); + if ( preamble != null ) +@@ -140,15 +155,11 @@ public class Shell + + if ( isQuotedExecutableEnabled() ) + { +- char[] escapeChars = +- getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() ); +- +- sb.append( StringUtils.quoteAndEscape( getExecutable(), getExecutableQuoteDelimiter(), escapeChars, +- getQuotingTriggerChars(), '\\', false ) ); ++ sb.append( quoteOneItem( executableParameter, true ) ); + } + else + { +- sb.append( getExecutable() ); ++ sb.append( executableParameter ); + } + } + for ( String argument : arguments ) +@@ -160,10 +171,7 @@ public class Shell + + if ( isQuotedArgumentsEnabled() ) + { +- char[] escapeChars = getEscapeChars( isSingleQuotedArgumentEscaped(), isDoubleQuotedArgumentEscaped() ); +- +- sb.append( StringUtils.quoteAndEscape( argument, getArgumentQuoteDelimiter(), escapeChars, +- getQuotingTriggerChars(), '\\', false ) ); ++ sb.append( quoteOneItem( argument, false ) ); + } + else + { +@@ -268,7 +276,7 @@ public class Shell + commandLine.addAll( getShellArgsList() ); + } + +- commandLine.addAll( getCommandLine( getExecutable(), arguments ) ); ++ commandLine.addAll( getCommandLine( executable, arguments ) ); + + return commandLine; + +@@ -368,4 +376,13 @@ public class Shell + this.singleQuotedExecutableEscaped = singleQuotedExecutableEscaped; + } + ++ public boolean isUnconditionalQuoting() ++ { ++ return unconditionalQuoting; ++ } ++ ++ public void setUnconditionalQuoting( boolean unconditionalQuoting ) ++ { ++ this.unconditionalQuoting = unconditionalQuoting; ++ } + } +diff --git a/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java b/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java +index 741f81c..5509cea 100644 +--- a/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java ++++ b/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java +@@ -45,7 +45,7 @@ public class BourneShellTest + + 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() +@@ -57,7 +57,7 @@ public class BourneShellTest + + 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() +@@ -69,7 +69,7 @@ public class BourneShellTest + + 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() +@@ -79,13 +79,13 @@ public class BourneShellTest + sh.setWorkingDirectory( "/usr/bin" ); + sh.setExecutable( "chmod" ); + +- String[] args = { "\'some arg with spaces\'" }; ++ String[] args = { "\"some arg with spaces\"" }; + + 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() +@@ -101,7 +101,21 @@ public class BourneShellTest + + String cli = StringUtils.join( shellCommandLine.iterator(), " " ); + System.out.println( cli ); +- assertTrue( cli.endsWith( "\'" + args[0] + "\'" ) ); ++ assertTrue( cli.endsWith("'some arg with spaces'")); ++ } ++ ++ public void testAddArgumentWithSingleQuote() ++ { ++ Shell sh = newShell(); ++ ++ sh.setWorkingDirectory( "/usr/bin" ); ++ sh.setExecutable( "chmod" ); ++ ++ String[] args = { "arg'withquote" }; ++ ++ List shellCommandLine = sh.getShellCommandLine( args ); ++ ++ assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1)); + } + + public void testArgumentsWithsemicolon() +@@ -120,7 +134,7 @@ public class BourneShellTest + + String cli = StringUtils.join( shellCommandLine.iterator(), " " ); + System.out.println( cli ); +- assertTrue( cli.endsWith( "\'" + args[0] + "\'" ) ); ++ assertTrue( cli.endsWith( "';some&argwithunix$chars'" ) ); + + Commandline commandline = new Commandline( newShell() ); + commandline.setExecutable( "chmod" ); +@@ -133,7 +147,7 @@ public class BourneShellTest + + assertEquals( "/bin/sh", lines.get( 0 ) ); + assertEquals( "-c", lines.get( 1 ) ); +- assertEquals( "chmod --password ';password'", lines.get( 2 ) ); ++ assertEquals( "'chmod' '--password' ';password'", lines.get( 2 ) ); + + commandline = new Commandline( newShell() ); + commandline.setExecutable( "chmod" ); +@@ -145,7 +159,7 @@ public class BourneShellTest + + assertEquals( "/bin/sh", lines.get( 0) ); + assertEquals( "-c", lines.get( 1 ) ); +- assertEquals( "chmod --password ';password'", lines.get( 2 ) ); ++ assertEquals( "'chmod' '--password' ';password'", lines.get( 2 ) ); + + commandline = new Commandline( new CmdShell() ); + commandline.getShell().setQuotedArgumentsEnabled( true ); +@@ -187,13 +201,14 @@ public class BourneShellTest + commandline.createArg().setValue( "{" ); + commandline.createArg().setValue( "}" ); + commandline.createArg().setValue( "`" ); ++ commandline.createArg().setValue( "#" ); + + List lines = commandline.getShell().getShellCommandLine( commandline.getArguments() ); + System.out.println( lines ); + + assertEquals( "/bin/sh", lines.get( 0 ) ); + assertEquals( "-c", lines.get( 1 ) ); +- assertEquals( "chmod ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'", ++ assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`' '#'", + lines.get( 2 ) ); + } + +-- +2.35.1 + diff --git a/SPECS/maven-shared-utils.spec b/SPECS/maven-shared-utils.spec index 290733a..2e27705 100644 --- a/SPECS/maven-shared-utils.spec +++ b/SPECS/maven-shared-utils.spec @@ -1,6 +1,6 @@ Name: maven-shared-utils Version: 0.4 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Maven shared utility classes License: ASL 2.0 URL: http://maven.apache.org/shared/maven-shared-utils @@ -8,6 +8,7 @@ Source0: http://repo1.maven.org/maven2/org/apache/maven/shared/%{name}/%{ # Patching tests so that they are compatible with JUnit 4.11 # (upstream bug http://jira.codehaus.org/browse/MSHARED-285) Patch0: %{name}-tests.patch +Patch1: 0001-Unconditionally-single-quote-executable-and-argument.patch BuildArch: noarch @@ -34,6 +35,7 @@ API documentation for %{name}. %setup -q %pom_remove_plugin org.codehaus.mojo:findbugs-maven-plugin %patch0 -p1 +%patch1 -p1 %build %mvn_build @@ -48,6 +50,10 @@ API documentation for %{name}. %doc LICENSE NOTICE %changelog +* Fri Apr 08 2022 Marian Koncek - 0.4-4 +- Fix commandline injection vulnerability +- Resolves: rhbz#2068651 + * Fri Dec 27 2013 Daniel Mach - 0.4-3 - Mass rebuild 2013-12-27