From: Francois Gouget <fgouget@codeweavers.com>
Subject: [PATCH 1/2] testbot/build: Quote the paths in the shell commands.
Message-Id: <91148018f00597a146a4fe81996f7ba0dc57a3ad.1529584617.git.fgouget@codeweavers.com>
Date: Thu, 21 Jun 2018 14:43:28 +0200 (CEST)

Also check that $DataDir does not contain single quotes, thus ensuring
our simple quoting scheme works.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---

A more absolutist approach would be to use ShQuote() everywhere which 
would remove the single quote restriction. But that would compromise 
readability. So this solution seems like a good compromise.

Also this is mostly cosmetic in the first place since the administrator 
would presumably not put spaces, stars, dollar signs or single quotes in 
the $DataDir path. Still I much prefer having proper quoting in my shell 
scripts.

 testbot/bin/build/Build.pl    | 16 +++++++++++-----
 testbot/bin/build/Reconfig.pl | 17 +++++++++++------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl
index c23bb8780..17ae74ff5 100755
--- a/testbot/bin/build/Build.pl
+++ b/testbot/bin/build/Build.pl
@@ -82,7 +82,7 @@ sub ApplyPatch($)
   my ($PatchFile) = @_;
 
   InfoMsg "Applying patch\n";
-  system("cd $DataDir/wine && set -x && ".
+  system("cd '$DataDir/wine' && set -x && ".
          "git apply --verbose ". ShQuote($PatchFile) ." && ".
          "git add -A");
   if ($? != 0)
@@ -95,7 +95,7 @@ sub ApplyPatch($)
   if ($Impacts->{Makefiles})
   {
     InfoMsg "\nRunning make_makefiles\n";
-    system("cd $DataDir/wine && set -x && ./tools/make_makefiles");
+    system("cd '$DataDir/wine' && set -x && ./tools/make_makefiles");
     if ($? != 0)
     {
       LogMsg "make_makefiles failed\n";
@@ -106,7 +106,7 @@ sub ApplyPatch($)
   if ($Impacts->{Autoconf} && !$Impacts->{HasConfigure})
   {
     InfoMsg "\nRunning autoconf\n";
-    system("cd $DataDir/wine && set -x && autoconf");
+    system("cd '$DataDir/wine' && set -x && autoconf");
     if ($? != 0)
     {
       LogMsg "Autoconf failed\n";
@@ -122,7 +122,7 @@ sub BuildNative()
   mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native");
 
   InfoMsg "\nRebuilding native tools\n";
-  system("cd $DataDir/build-native && set -x && ".
+  system("cd '$DataDir/build-native' && set -x && ".
          "time make -j$ncpus __tooldeps__");
   if ($? != 0)
   {
@@ -147,7 +147,7 @@ sub BuildTestExecutables($$)
   }
 
   InfoMsg "\nBuilding the $Bits-bit test executable(s)\n";
-  system("cd $DataDir/build-mingw$Bits && set -x && ".
+  system("cd '$DataDir/build-mingw$Bits' && set -x && ".
          "time make -j$ncpus ". join(" ", sort @BuildDirs));
   if ($? != 0)
   {
@@ -226,6 +226,12 @@ else
   FatalError "Invalid number of bits $BitIndicators\n";
 }
 
+if ($DataDir =~ /'/)
+{
+    LogMsg "The install path contains invalid characters\n";
+    exit(1);
+}
+
 my $Impacts = ApplyPatch($PatchFile);
 exit(1) if (!$Impacts);
 
diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl
index 2da2f54a4..5ecbc2af3 100755
--- a/testbot/bin/build/Reconfig.pl
+++ b/testbot/bin/build/Reconfig.pl
@@ -73,7 +73,7 @@ sub BuildTestAgentd()
   if (! -x "$BinDir/build/testagentd")
   {
     InfoMsg "\nBuilding the native testagentd\n";
-    system("cd $::RootDir/src/testagentd && set -x && ".
+    system("cd '$::RootDir/src/testagentd' && set -x && ".
            "time make -j$ncpus build");
     if ($? != 0)
     {
@@ -83,7 +83,7 @@ sub BuildTestAgentd()
   }
 
   InfoMsg "\nRebuilding the Windows TestAgentd\n";
-  system("cd $::RootDir/src/testagentd && set -x && ".
+  system("cd '$::RootDir/src/testagentd' && set -x && ".
          "time make -j$ncpus iso");
   if ($? != 0)
   {
@@ -97,7 +97,7 @@ sub BuildTestAgentd()
 sub BuildTestLauncher()
 {
   InfoMsg "\nRebuilding TestLauncher\n";
-  system("cd $::RootDir/src/TestLauncher && set -x && ".
+  system("cd '$::RootDir/src/TestLauncher' && set -x && ".
          "time make -j$ncpus");
   if ($? != 0)
   {
@@ -111,7 +111,7 @@ sub BuildTestLauncher()
 sub GitPull()
 {
   InfoMsg "\nUpdating the Wine source\n";
-  system("cd $DataDir/wine && git pull");
+  system("cd '$DataDir/wine' && git pull");
   if ($? != 0)
   {
     LogMsg "Git pull failed\n";
@@ -134,7 +134,7 @@ sub BuildNative()
 
   # Rebuild from scratch to make sure cruft will not accumulate
   InfoMsg "\nRebuilding native tools\n";
-  system("cd $DataDir/build-native && set -x && ".
+  system("cd '$DataDir/build-native' && set -x && ".
          "rm -rf * && ".
          "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ".
          "time make -j$ncpus __tooldeps__");
@@ -157,7 +157,7 @@ sub BuildCross($)
 
   # Rebuild from scratch to make sure cruft will not accumulate
   InfoMsg "\nRebuilding the $Bits-bit test executables\n";
-  system("cd $DataDir/build-mingw$Bits && set -x && ".
+  system("cd '$DataDir/build-mingw$Bits' && set -x && ".
          "rm -rf * && ".
          "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ".
          "time make -j$ncpus buildtests");
@@ -173,6 +173,11 @@ sub BuildCross($)
 $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin";
 delete $ENV{ENV};
 
+if ($DataDir =~ /'/)
+{
+    LogMsg "The install path contains invalid characters\n";
+    exit(1);
+}
 if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging")
 {
     LogMsg "Unable to create '$DataDir/staging': $!\n";

-- 
2.17.1