Fix Debian bug #478942: Fragile argument passing

Fix some problems regarding the way check_disk_smb passes command line
arguments to smbclient(1).

| It runs:
|
| 	$res = qx/$smbclient "\/\/$host\/$share" $pass -W $workgroup \
| 	       -U $user $smbclientoptions -I $address -c ls/;
|
| [...]
|
| The documentation says that if the password is not passed, it
| defaults to "".  That is not true above, as $pass expands to
| nothing which leaves no argument at all (instead of an empty
| argument) so is different from providing with an empty password
| or with the -N option.
|
| Also, if the password starts with "-", you're in trouble, that's
| why -U $user%$pass may be prefered.
|
| Also, the doc says that if $user is not provided, then it
| defaults to "guest" but the problem is that if it is provided
| but empty, it is changed to "guest" as well, which prevents us
| from querying hosts that don't do user authentication.

[ http://bugs.debian.org/478942 ]

(Fixed by Stephane Chazelas, forwarded by Jan Wagner.)

diff --git a/plugins-scripts/check_disk_smb.pl b/plugins-scripts/check_disk_smb.pl
index ca593d468480bb9384324914d530cef495339992..7c81fc263a82f220e65cffe4b0bbf30fa88246bd 100755 (executable)
--- a/plugins-scripts/check_disk_smb.pl
+++ b/plugins-scripts/check_disk_smb.pl
@@ -58,9 +58,7 @@ if ($opt_V) {
 
 if ($opt_h) {print_help(); exit $ERRORS{'OK'};}
 
 
 if ($opt_h) {print_help(); exit $ERRORS{'OK'};}
 

-my $smbclient= "$utils::PATH_TO_SMBCLIENT " ;
-my $smbclientoptions= $opt_P ? "-p $opt_P " : "";
-
+my $smbclient = $utils::PATH_TO_SMBCLIENT;

 
 # Options checking
 
 
 # Options checking
 


@@ -72,13 +70,12 @@ my $host = $1 if ($opt_H =~ /^([-_.A-Za-z0-9 ]+\$?)$/);
 my $share = $1 if ($opt_s =~ /^([-_.A-Za-z0-9]+\$?)$/);
 ($share) || usage("Invalid share: $opt_s\n");
 
 my $share = $1 if ($opt_s =~ /^([-_.A-Za-z0-9]+\$?)$/);
 ($share) || usage("Invalid share: $opt_s\n");
 

-($opt_u) || ($opt_u = shift @ARGV) || ($opt_u = "guest");
-my $user = $1 if ($opt_u =~ /^([-_.A-Za-z0-9\\]+)$/);
-($user) || usage("Invalid user: $opt_u\n");
+defined($opt_u) || ($opt_u = shift @ARGV) || ($opt_u = "guest");
+my $user = $1 if ($opt_u =~ /^([-_.A-Za-z0-9\\]*)$/);
+defined($user) || usage("Invalid user: $opt_u\n");

 
 

-($opt_p) || ($opt_p = shift @ARGV) || ($opt_p = "");
+defined($opt_p) || ($opt_p = shift @ARGV) || ($opt_p = "");

 my $pass = $1 if ($opt_p =~ /(.*)/);
 my $pass = $1 if ($opt_p =~ /(.*)/);

-$pass = "-N" if ($opt_p eq "");

 
 ($opt_w) || ($opt_w = shift @ARGV) || ($opt_w = 85);
 my $warn = $1 if ($opt_w =~ /^([0-9]{1,2}\%?|100\%?|[0-9]+[kMG])$/);
 
 ($opt_w) || ($opt_w = shift @ARGV) || ($opt_w = 85);
 my $warn = $1 if ($opt_w =~ /^([0-9]{1,2}\%?|100\%?|[0-9]+[kMG])$/);


@@ -88,6 +85,24 @@ my $warn = $1 if ($opt_w =~ /^([0-9]{1,2}\%?|100\%?|[0-9]+[kMG])$/);
 my $crit = $1 if ($opt_c =~ /^([0-9]{1,2}\%?|100\%?|[0-9]+[kMG])$/);
 ($crit) || usage("Invalid critical threshold: $opt_c\n");
 
 my $crit = $1 if ($opt_c =~ /^([0-9]{1,2}\%?|100\%?|[0-9]+[kMG])$/);
 ($crit) || usage("Invalid critical threshold: $opt_c\n");
 

+# Execute the given command line and return anything it writes to STDOUT and/or
+# STDERR.  (This might be useful for other plugins, too, so it should possibly
+# be moved to utils.pm.)
+sub output_and_error_of {
+       local *CMD;
+       local $/ = undef;
+       my $pid = open CMD, "-|";
+       if (defined($pid)) {
+               if ($pid) {
+                       return <CMD>;
+               } else {
+                       open STDERR, ">&STDOUT" and exec @_;
+                       exit(1);
+               }
+       }
+       return undef;
+}
+

 # split the type from the unit value
 #Check $warn and $crit for type (%/M/G) and set up for tests
 #P = Percent, K = KBytes
 # split the type from the unit value
 #Check $warn and $crit for type (%/M/G) and set up for tests
 #P = Percent, K = KBytes


@@ -162,23 +177,19 @@ alarm($TIMEOUT);
 
 # Execute an "ls" on the share using smbclient program
 # get the results into $res
 
 # Execute an "ls" on the share using smbclient program
 # get the results into $res

-if (defined($workgroup)) {
-       if (defined($address)) {
-               print "$smbclient " . "\/\/$host\/$share" ." $pass -W $workgroup -U $user $smbclientoptions -I $address -c ls\n" if ($verbose);
-               $res = qx/$smbclient "\/\/$host\/$share" $pass -W $workgroup -U $user $smbclientoptions -I $address -c ls/;
-       } else {
-               print "$smbclient " . "\/\/$host\/$share" ." $pass -W $workgroup -U $user $smbclientoptions -c ls\n" if ($verbose);
-               $res = qx/$smbclient "\/\/$host\/$share" $pass -W $workgroup -U $user $smbclientoptions -c ls/;
-       }
-} else {
-       if (defined($address)) {
-               print "$smbclient " . "\/\/$host\/$share" ." $pass -U $user $smbclientoptions -I $address -c ls\n" if ($verbose);
-               $res = qx/$smbclient "\/\/$host\/$share" $pass -U $user $smbclientoptions -I $address -c ls/;
-       } else {
-               print "$smbclient " . "\/\/$host\/$share" ." $pass -U $user $smbclientoptions -c ls\n" if ($verbose);
-               $res = qx/$smbclient "\/\/$host\/$share" $pass -U $user $smbclientoptions -c ls/;
-       }
-}
+my @cmd = (
+       $smbclient,
+       "//$host/$share",
+       "-U", "$user%$pass",
+       defined($workgroup) ? ("-W", $workgroup) : (),
+       defined($address) ? ("-I", $address) : (),
+       defined($opt_P) ? ("-p", $opt_P) : (),
+       "-c", "ls"
+);
+
+print join(" ", @cmd) . "\n" if ($verbose);
+$res = output_and_error_of(@cmd) or exit $ERRORS{"UNKNOWN"};
+

 #Turn off alarm
 alarm(0);
 
 #Turn off alarm
 alarm(0);