Module: tools
Branch: master
Commit: 62342aa288bfaacafb0c5a52f125fae0089e9e1e
URL: https://source.winehq.org/git/tools.git/?a=commit;h=62342aa288bfaacafb0c5a5…
Author: Francois Gouget <fgouget(a)codeweavers.com>
Date: Wed Jun 20 02:43:59 2018 +0200
testbot/Build: Don't use tainting and improve handling of the patch filename.
Tainting is moot because the whole purpose of the script is to run
arbitrary user-provided code (in patch form).
Still validate the patch filename but use the standard IsValidFileName()
function (instead of the much stricter regular expression used so far)
so patches accepted by Submit.pl don't get rejected at this late stage.
However carefully quote the filename when building our shell command so
it does not fail if the filename contains a space (for instance).
Also note that in practice (so far) the patch filename is always called
patch.diff as this is hardcoded in WineRunBuild.pl. So this is all a
bit academic.
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>
---
testbot/bin/build/Build.pl | 25 ++++++++++++++-----------
testbot/lib/WineTestBot/Utils.pm | 26 +++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl
index 8318af1..e979955 100755
--- a/testbot/bin/build/Build.pl
+++ b/testbot/bin/build/Build.pl
@@ -1,10 +1,13 @@
-#!/usr/bin/perl -Tw
+#!/usr/bin/perl
# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*-
#
# Performs the 'build' task in the build machine. Specifically this applies a
# conformance test patch, rebuilds the impacted test and retrieves the
# resulting 32 and 64 bit binaries.
#
+# This script does not use tainting (-T) because its whole purpose is to run
+# arbitrary user-provided code anyway (in patch form).
+#
# Copyright 2009 Ge van Geldorp
# Copyright 2012-2014, 2017-2018 Francois Gouget
#
@@ -22,6 +25,7 @@
# License along with this library; if not, write to the Free Software
# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+use warnings;
use strict;
sub BEGIN
@@ -42,6 +46,8 @@ sub BEGIN
use WineTestBot::Config;
use WineTestBot::PatchUtils;
+use WineTestBot::Utils;
+
my $LogFileName = "$LogDir/Build.log";
@@ -85,7 +91,7 @@ sub ApplyPatch($)
InfoMsg "Applying patch\n";
system("( cd $DataDir/wine && set -x && " .
- " git apply --verbose $PatchFile && " .
+ " git apply --verbose ". ShQuote($PatchFile) ." && ".
" git add -A " .
") >>$LogFileName 2>&1");
if ($? != 0)
@@ -199,18 +205,15 @@ if (! $PatchFile || !$BitIndicators)
FatalError "Usage: Build.pl <patchfile> <bits>\n";
}
-# Untaint parameters
-if ($PatchFile =~ m/^([\w_.\-]+)$/)
+# Verify parameters
+if (!IsValidFileName($PatchFile))
{
- $PatchFile = "$DataDir/staging/$1";
- if (! -r $PatchFile)
- {
- FatalError "Patch file $PatchFile not readable\n";
- }
+ FatalError "The patch filename '$PatchFile' contains invalid characters\n";
}
-else
+$PatchFile = "$DataDir/staging/$PatchFile";
+if (!-r $PatchFile)
{
- FatalError "Invalid patch file $PatchFile\n";
+ FatalError "Patch file '$PatchFile' is not readable\n";
}
my ($Run32, $Run64);
diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm
index 962e6ff..111a565 100644
--- a/testbot/lib/WineTestBot/Utils.pm
+++ b/testbot/lib/WineTestBot/Utils.pm
@@ -28,7 +28,8 @@ WineTestBot::Utils - Utility functions
use Exporter 'import';
our @EXPORT = qw(MakeSecureURL SecureConnection GenerateRandomString
OpenNewFile CreateNewFile CreateNewLink CreateNewDir
- DurationToString BuildEMailRecipient IsValidFileName);
+ DurationToString BuildEMailRecipient IsValidFileName
+ ShQuote);
use Fcntl;
@@ -196,4 +197,27 @@ sub IsValidFileName($)
return $FileName !~ m~[<>:"/\\|?*]~;
}
+=pod
+=over 12
+
+=item C<ShQuote()>
+
+Quotes strings so they can be used in shell commands.
+
+Note that this implies escaping '$'s and '`'s which may not be appropriate
+in another context.
+
+=back
+=cut
+
+sub ShQuote($)
+{
+ my ($Str)=@_;
+ $Str =~ s%\\%\\\\%g;
+ $Str =~ s%\$%\\\$%g;
+ $Str =~ s%\"%\\\"%g;
+ $Str =~ s%\`%\\\`%g;
+ return "\"$Str\"";
+}
+
1;
Module: tools
Branch: master
Commit: 759fc680db14c71e66e4ca5afb141c85e0504308
URL: https://source.winehq.org/git/tools.git/?a=commit;h=759fc680db14c71e66e4ca5…
Author: Francois Gouget <fgouget(a)codeweavers.com>
Date: Wed Jun 20 02:43:45 2018 +0200
testbot/web: Reject filenames that are not valid Windows filenames.
IsValidFileName() verifies that the filename is valid on both Windows
and Unix. This is necessary to ensure we will be able to upload the file
to the build and/or test VMs.
IsValidFileName() is defined in the Utils.pm module so it can be reused
where necessary.
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>
---
testbot/lib/WineTestBot/Utils.pm | 25 ++++++++++++++++++++++++-
testbot/web/Submit.pl | 4 ++--
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm
index 8b0bfeb..962e6ff 100644
--- a/testbot/lib/WineTestBot/Utils.pm
+++ b/testbot/lib/WineTestBot/Utils.pm
@@ -28,7 +28,7 @@ WineTestBot::Utils - Utility functions
use Exporter 'import';
our @EXPORT = qw(MakeSecureURL SecureConnection GenerateRandomString
OpenNewFile CreateNewFile CreateNewLink CreateNewDir
- DurationToString BuildEMailRecipient);
+ DurationToString BuildEMailRecipient IsValidFileName);
use Fcntl;
@@ -173,4 +173,27 @@ sub CreateNewDir($$)
}
}
+
+#
+# Shell helpers
+#
+
+=pod
+=over 12
+
+=item C<IsValidFileName()>
+
+Returns true if the filename is valid on Unix and Windows systems.
+
+This also ensures this is not a trick filename such as '../important/file'.
+
+=back
+=cut
+
+sub IsValidFileName($)
+{
+ my ($FileName) = @_;
+ return $FileName !~ m~[<>:"/\\|?*]~;
+}
+
1;
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index c16b99f..afebc72 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -514,10 +514,10 @@ sub ValidateAndGetFileName($$)
$self->{ErrMessage} = "You must provide a file to test";
return undef;
}
- if ($FileName =~ m=[/\\]=)
+ if (!IsValidFileName($FileName))
{
$self->{ErrField} = $FieldName;
- $self->{ErrMessage} = "The filename is invalid";
+ $self->{ErrMessage} = "The filename contains invalid characters";
return undef;
}
my $PropertyDescriptor = CreateSteps()->GetPropertyDescriptorByName("FileName");
Module: tools
Branch: master
Commit: fea386748c25f865009831611de4c93d4ff3a2f7
URL: https://source.winehq.org/git/tools.git/?a=commit;h=fea386748c25f8650098316…
Author: Francois Gouget <fgouget(a)codeweavers.com>
Date: Wed Jun 20 02:31:27 2018 +0200
testbot: Adjust the reconfig timeout.
$ReconfigTimeout was way overestimated, maybe reflecting a time when
either the TestBot or the Wine dependencies were causing too many files
to be rebuilt.
Note that $BuildTimeout is a bit large too: a patch touching a test
module and a core header would typically only require about 2.5
minutes. But in the unlikely event where that patch touched dozens of
test modules it may still be too short. So $BuildTimeout is left
unchanged.
Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>
---
testbot/lib/WineTestBot/Config.pm | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/testbot/lib/WineTestBot/Config.pm b/testbot/lib/WineTestBot/Config.pm
index b7d633d..66cb813 100644
--- a/testbot/lib/WineTestBot/Config.pm
+++ b/testbot/lib/WineTestBot/Config.pm
@@ -97,9 +97,10 @@ $SingleTimeout = 2 * 60;
# How long to let a regular build run before forcibly shutting it down
# (in seconds).
$BuildTimeout = 5 * 60;
-# How long to let a full recompilation run before forcibly shutting it down
-# (in seconds).
-$ReconfigTimeout = 45 * 60;
+# How long to let a reconfig task run before forcibly shutting it down
+# (in seconds). Note that this includes building the native Wine build tools,
+# and the 32 and 64 bit test executables.
+$ReconfigTimeout = (1 + 2 * 5) * 60;
# How much to add to the task timeout to account for file transfers, etc.
$TimeoutMargin = 2 * 60;
# Maximum amount of traces for a test unit.