The following commit has been merged in the master branch: commit 7b79d64a9721db92a02309562244d3bb61d40a5f Author: Raphael Hertzog <hert...@debian.org> Date: Sat Feb 21 22:22:29 2009 +0100
update-alternatives: add more checks on --install parameters Check that links and alternative paths are absolute. Check that the master alternative path exists. Forbid "/" and spaces in alternative names. Closes: #423176 Add corresponding tests in the test-suite. diff --git a/ChangeLog b/ChangeLog index c3682fb..d9b42de 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2009-02-22 Raphael Hertzog <hert...@debian.org> + * scripts/update-alternatives.pl: Add more sanity check for + --install: ensure alternative path and links are absolute, ensure + master alternative path exists, forbid / and spaces in alternative + names. + * scripts/t/900_update_alternatives.t: Add corresponding tests in + the test suite. + +2009-02-22 Raphael Hertzog <hert...@debian.org> + * scripts/update-alternatives.pl: Checks that --install does not reuse alternatives or links in a way that is not compatible with the current setup. diff --git a/debian/changelog b/debian/changelog index df96417..ce32c5c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -160,6 +160,7 @@ dpkg (1.15.0) UNRELEASED; urgency=low - it forbids reusing master alternative as slave and vice-versa. Closes: #342566 - it forbids reusing alternative links managed by other alternatives + - new sanity checks on --install parameters. Closes: #423176 [ Pierre Habouzit ] * Add a --query option to update-alternatives. Closes: #336091, #441904 diff --git a/scripts/t/900_update_alternatives.t b/scripts/t/900_update_alternatives.t index 4d1004b..997d663 100644 --- a/scripts/t/900_update_alternatives.t +++ b/scripts/t/900_update_alternatives.t @@ -53,7 +53,7 @@ my @choices = ( ); my $nb_slaves = 2; plan tests => (4 * ($nb_slaves + 1) + 2) * 18 # number of check_choices - + 44; # rest + + 49; # rest sub cleanup { system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p $altdir"); @@ -281,5 +281,21 @@ call_ua(["--install", "$bindir/slave1", "testmaster", "/bin/date", "10"], call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10", "--slave", "$bindir/generic-test", "testslave", "/bin/true" ], expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# lack of absolute filenames in links or file path, non-existing path, +call_ua(["--install", "../testmaster", "testmaster", "/bin/date", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +call_ua(["--install", "$bindir/testmaster", "testmaster", "./update-alternatives.pl", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# non-existing alternative path +call_ua(["--install", "$bindir/testmaster", "testmaster", "$bindir/doesntexist", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# invalid alternative name in master +call_ua(["--install", "$bindir/testmaster", "test/master", "/bin/date", "10"], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); +# invalid alternative name in slave +call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10", + "--slave", "$bindir/testslave", "test slave", "/bin/true" ], + expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null"); -# TODO: add checks for invalid values of parameters, install in non-existing dir +# TODO: install in non-existing dir, handle of pre-existing files in place +# of alternative links diff --git a/scripts/update-alternatives.pl b/scripts/update-alternatives.pl index 2f5bb35..cdf6675 100755 --- a/scripts/update-alternatives.pl +++ b/scripts/update-alternatives.pl @@ -122,9 +122,10 @@ foreach my $alt_name (get_all_alternatives()) { } } # Check that caller don't mix links between alternatives and don't mix -# alternatives between +# alternatives between slave/master, and that the various parameters +# are fine if ($action eq "install") { - my ($name, $link) = ($inst_alt->name(), $inst_alt->link()); + my ($name, $link, $file) = ($inst_alt->name(), $inst_alt->link(), $fileset->master()); if (exists $ALL{parent}{$name} and $ALL{parent}{$name} ne $name) { badusage(_g("Alternative %s can't be master: %s"), $name, sprintf(_g("it is a slave of %s"), $ALL{parent}{$name})); @@ -133,8 +134,16 @@ if ($action eq "install") { badusage(_g("Alternative link %s is already managed by %s."), $link, $ALL{parent}{$ALL{links}{$link}}); } + badusage(_g("Alternative link (%s) is not absolute as it should be."), + $link) unless $link =~ m|^/|; + badusage(_g("Alternative path (%s) is not absolute as it should be."), + $file) unless $file =~ m|^/|; + badusage(_g("Alternative path (%s) doesn't exist."), $file) + unless -e $file; + badusage(_g("Alternative name (%s) is invalid."), $name) if $name =~ m|[/\s]|; foreach my $slave ($inst_alt->slaves()) { $link = $inst_alt->slave_link($slave); + $file = $fileset->slave($slave); if (exists $ALL{parent}{$slave} and $ALL{parent}{$slave} ne $name) { badusage(_g("Alternative %s can't be slave of %s: %s"), $slave, $name, ($ALL{parent}{$slave} eq $slave) ? @@ -146,6 +155,12 @@ if ($action eq "install") { badusage(_g("Alternative link %s is already managed by %s."), $link, $ALL{parent}{$ALL{links}{$link}}); } + badusage(_g("Alternative link (%s) is not absolute as it should be."), + $link) unless $link =~ m|^/|; + badusage(_g("Alternative path (%s) is not absolute as it should be."), + $file) unless $file =~ m|^/|; + badusage(_g("Alternative name (%s) is invalid."), $slave) + if $slave =~ m|[/\s]|; } } -- dpkg's main repository -- To UNSUBSCRIBE, email to debian-dpkg-cvs-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org