[debhelper-devel] [debhelper] 01/01: dh_installdeb: Basic validation of {rm, mv}_conffile

Niels Thykier nthykier at moszumanska.debian.org
Sat Nov 25 12:02:58 UTC 2017


This is an automated email from the git hooks/post-receive script.

nthykier pushed a commit to branch master
in repository debhelper.

commit 6c634c8067ac6426da5c76b0be4b2b7dce2f3ecb
Author: Niels Thykier <niels at thykier.net>
Date:   Sat Nov 25 12:01:20 2017 +0000

    dh_installdeb: Basic validation of {rm,mv}_conffile
    
    Signed-off-by: Niels Thykier <niels at thykier.net>
---
 debhelper.pod                  |  6 ++++++
 debian/changelog               |  9 ++++++++
 dh_installdeb                  | 49 ++++++++++++++++++++++++++++++++++++++++++
 lib/Debian/Debhelper/Dh_Lib.pm |  7 ++++++
 t/maintscript.t                | 36 ++++++++++++++++++++++++++++++-
 5 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/debhelper.pod b/debhelper.pod
index 1c60bf5..00171e2 100644
--- a/debhelper.pod
+++ b/debhelper.pod
@@ -760,6 +760,12 @@ finds an old upstart configuration file.  The error is there to
 remind the package maintainer to ensure the proper removal of the
 conffiles shipped in previous versions of the package (if any).
 
+=item -
+
+The B<dh_installdeb> tool will do basic validation of some
+L<dpkg-maintscript-helper(1)> commands and will error out if the
+commands appear to be invalid.
+
 =back
 
 =back
diff --git a/debian/changelog b/debian/changelog
index 7d47679..a1c6814 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+debhelper (10.10.10) UNRELEASED; urgency=medium
+
+  * dh_installdeb: Do basic validation of rm_conffile and mv_conffile
+    parameters in "maintscript" parameters in compat 10 and newer.
+    In compat 12, this is an error.  Thanks to Andreas Beckmann for
+    the suggestions.  (Closes: #882626)
+
+ -- Niels Thykier <niels at thykier.net>  Sat, 25 Nov 2017 11:59:53 +0000
+
 debhelper (10.10.9) unstable; urgency=medium
 
   [ Dmitry Shachnev ]
diff --git a/dh_installdeb b/dh_installdeb
index 8dc0bd6..e25a472 100755
--- a/dh_installdeb
+++ b/dh_installdeb
@@ -79,6 +79,11 @@ It was also the intention to escape shell metacharacters in previous
 compat levels.  However, it did not work properly and as such it was
 possible to embed arbitrary shell code in earlier compat levels.
 
+The B<dh_installdeb> tool will do some basic validation of some of
+the commands listed in this file to catch common mistakes.  The
+validation is enabled as a warning since compat 10 and as a hard
+error in compat 12.
+
 =back
 
 =cut
@@ -93,6 +98,10 @@ my %maintscript_predeps = (
 	"symlink_to_dir" => "",
 	"dir_to_symlink" => "",
 );
+my %maintscript_validator = (
+	"rm_conffile" => \&_validate_conffile_args,
+	"mv_conffile" => \&_validate_conffile_args,
+);
 
 foreach my $package (@{$dh{DOPACKAGES}}) {
 	my $tmp=tmpdir($package);
@@ -139,6 +148,9 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 				addsubstvar($package, "misc:Pre-Depends", "dpkg",
 					">= $maintscript_predeps{$cmd}")
 					if length $maintscript_predeps{$cmd};
+				if (my $validator = $maintscript_validator{$cmd}) {
+					$validator->($package, @{$line});
+				}
 				push(@params, escape_shell(@{$line}) );
 			}
 			foreach my $script (qw{postinst preinst prerm postrm}) {
@@ -214,6 +226,43 @@ sub install_triggers {
 	install_file($sources[0], $target);
 }
 
+sub _validate_conffile_args {
+	my ($package, $cmd, @args) = @_;
+	my ($current_conffile, $new_conffile, $prior_version, $owning_package, $other);
+	for my $arg (@args) {
+		if ($arg eq '--') {
+			_maybe_error("The maintscripts file for $package includes a \"--\" for one of the ${cmd} commands, but it should not");
+		}
+	}
+	if ($cmd eq 'rm_conffile') {
+		($current_conffile, $prior_version, $owning_package, $other) = @args;
+	} else {
+		($current_conffile, $new_conffile, $prior_version, $owning_package, $other) = @args;
+	}
+	$current_conffile //= '';
+	_maybe_error("The current conffile path for ${cmd} must be present and absolute, got ${current_conffile}")
+		if not $current_conffile or substr($current_conffile, 0, 1) ne '/';
+	_maybe_error("The new conffile path for ${cmd} must be present and absolute, got ${new_conffile}")
+		if $cmd eq 'mv_conffile' and (not $new_conffile or substr($new_conffile, 0, 1) ne '/');
+
+	_maybe_error("The version for ${cmd} ${current_conffile} is not valid, got ${prior_version}")
+		if $prior_version and $prior_version !~ m{^${Debian::Debhelper::Dh_Lib::PKGVERSION_REGEX}$}o;
+	_maybe_error("The owning package for ${cmd} ${current_conffile} is not valid, got ${owning_package}")
+		if $owning_package and $owning_package !~ m{^${Debian::Debhelper::Dh_Lib::PKGNAME_REGEX}$}o;
+	if (defined($other)) {
+		warning("Too many arguments for ${cmd} ${current_conffile}");
+	}
+}
+
+sub _maybe_error {
+	my ($msg) = @_;
+	if (compat(11)) {
+		warning($msg);
+	} else {
+		error($msg);
+	}
+}
+
 =head1 SEE ALSO
 
 L<debhelper(7)>
diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
index 37be1cc..b44ca7e 100644
--- a/lib/Debian/Debhelper/Dh_Lib.pm
+++ b/lib/Debian/Debhelper/Dh_Lib.pm
@@ -78,6 +78,13 @@ my $prefix="/usr";
 my $MAX_PROCS = get_buildoption("parallel") || 1;
 my $DH_TOOL_VERSION;
 
+our $PKGNAME_REGEX = qr/[a-z0-9][-+\.a-z0-9]+/o;
+our $PKGVERSION_REGEX = qr/
+                 (?: \d+ : )?                # Optional epoch
+                 [0-9][0-9A-Za-z.+:~]*       # Upstream version (with no hyphens)
+                 (?: - [0-9A-Za-z.+:~]+ )*   # Optional debian revision (+ upstreams versions with hyphens)
+                          /xoa;
+
 sub init {
 	my %params=@_;
 
diff --git a/t/maintscript.t b/t/maintscript.t
index a69c99c..4460f2c 100755
--- a/t/maintscript.t
+++ b/t/maintscript.t
@@ -10,7 +10,7 @@ use Test::DH;
 use Debian::Debhelper::Dh_Lib qw(!dirname);
 
 if (uid_0_test_is_ok()) {
-	plan(tests => 1);
+	plan(tests => 2);
 } else {
 	plan skip_all => 'fakeroot required';
 }
@@ -38,3 +38,37 @@ EOF
 	}
 });
 
+sub test_maintscript_syntax {
+	my ($contents) = @_;
+	my @scripts = map { ("debian/debhelper.${_}.debhelper", "debian/$_") } qw{postinst preinst prerm postrm};
+	my $file = 'debian/maintscript';
+
+
+	open(my $fd, ">", $file) or die("open($file): $!");
+	print {$fd} <<EOF;
+${contents}
+EOF
+	close($fd) or die("close($file): $!\n");
+
+	my $res = run_dh_tool( { 'needs_root' => 1, 'quiet' => 1 }, 'dh_installdeb');
+
+	remove_tree('debian/debhelper', 'debian/tmp', 'debian/.debhelper');
+	rm_files(@scripts);
+
+	return $res;
+}
+
+# Negative tests
+each_compat_from_and_above_subtest(12, sub {
+	ok(!test_maintscript_syntax('rm_conffile foo 1.0~'), "rm_conffile absolute path check");
+	ok(!test_maintscript_syntax('rm_conffile /foo 1.0\~'), "rm_conffile version check");
+	ok(!test_maintscript_syntax('rm_conffile /foo 1.0~ some_pkg'), "rm_conffile package name check");
+	ok(!test_maintscript_syntax('rm_conffile /foo 1.0~ some-pkg --'), "rm_conffile separator check");
+
+	ok(!test_maintscript_syntax('mv_conffile foo /bar 1.0~'), "mv_conffile absolute (current) path check");
+	ok(!test_maintscript_syntax('mv_conffile /foo bar 1.0~'), "mv_conffile absolute (current) path check");
+	ok(!test_maintscript_syntax('mv_conffile /foo /bar 1.0\~'), "mv_conffile version check");
+	ok(!test_maintscript_syntax('mv_conffile /foo /bar 1.0~ some_pkg'), "mv_conffile package name check");
+	ok(!test_maintscript_syntax('mv_conffile /foo /bar 1.0~ some-pkg -- '), "mv_conffile separator check ");
+});
+

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debhelper/debhelper.git




More information about the debhelper-devel mailing list