do shell_quote-ing after STDIN strings are evaluated

1 files changed, 37 insertions, 26 deletions

diff --git a/FS/FS/part_export/shellcommands.pm b/FS/FS/part_export/shellcommands.pm
index 6e9168a6d..0545b5c59 100644
--- a/FS/FS/part_export/shellcommands.pm
+++ b/FS/FS/part_export/shellcommands.pm
@@ -156,13 +156,13 @@ old_ for replace operations):
 <UL>
   <LI><code>$username</code>
   <LI><code>$_password</code>
-  <LI><code>$quoted_password</code> - unencrypted password, already quoted for the shell (do not add additional quotes)
-  <LI><code>$crypt_password</code> - encrypted password, already quoted for the shell (do not add additional quotes)
+  <LI><code>$quoted_password</code> - unencrypted password, already quoted for the shell (do not add additional quotes).
+  <LI><code>$crypt_password</code> - encrypted password.  When used on the command line (rather than STDIN), it will be already quoted for the shell (do not add additional quotes).
   <LI><code>$uid</code>
   <LI><code>$gid</code>
-  <LI><code>$finger</code> - GECOS, already quoted for the shell (do not add additional quotes)
-  <LI><code>$first</code> - First name of GECOS, already quoted for the shell (do not add additional quotes)
-  <LI><code>$last</code> - Last name of GECOS, already quoted for the shell (do not add additional quotes)
+  <LI><code>$finger</code> - GECOS.  When used on the command line (rather than STDIN), it will be already quoted for the shell (do not add additional quotes).
+  <LI><code>$first</code> - First name of GECOS.  When used on the command line (rather than STDIN), it will be already quoted for the shell (do not add additional quotes).
+  <LI><code>$last</code> - Last name of GECOS.  When used on the command line (rather than STDIN), it will be already quoted for the shell (do not add additional quotes).
   <LI><code>$dir</code> - home directory
   <LI><code>$shell</code>
   <LI><code>$quota</code>
@@ -215,7 +215,6 @@ sub _export_command_or_super {
   }
 };
 
-
 sub _export_command {
   my ( $self, $action, $svc_acct) = (shift, shift, shift);
   my $command = $self->option($action);
@@ -227,6 +226,7 @@ sub _export_command {
     no strict 'refs';
     ${$_} = $svc_acct->getfield($_) foreach $svc_acct->fields;
 
+    # snarfs are unused at this point?
     my $count = 1;
     foreach my $acct_snarf ( $svc_acct->acct_snarf ) {
       ${"snarf_$_$count"} = shell_quote( $acct_snarf->get($_) )
@@ -244,19 +244,16 @@ sub _export_command {
 
   $finger =~ /^(.*)\s+(\S+)$/ or $finger =~ /^((.*))$/;
   ($first, $last ) = ( $1, $2 );
-  $first = shell_quote $first;
-  $last = shell_quote $last;
-  $finger = shell_quote $finger;
-  $quoted_password = shell_quote $_password;
   $domain = $svc_acct->domain;
 
-  $crypt_password =
-    shell_quote( $svc_acct->crypt_password( $self->option('crypt') ) );
+  $quoted_password = shell_quote $_password;
+
+  $crypt_password = $svc_acct->crypt_password( $self->option('crypt') );
 
   @radius_groups = $svc_acct->radius_groups;
 
   my ($reasonnum, $reasontext, $reasontypenum, $reasontypetext);
-  if ( $cust_pkg && $action eq 'suspend' && (my $r = $cust_pkg->last_reason)){
+  if ( $cust_pkg && $action eq 'suspend' && (my $r = $cust_pkg->last_reason) ) {
     $reasonnum = $r->reasonnum;
     $reasontext = $r->reason;
     $reasontypenum = $r->reason_type;
@@ -270,29 +267,38 @@ sub _export_command {
       if (!$userspec && exists($reasonmap{$reasontext}));
 
     my $suspend_user;
-    if ($userspec =~ /^\d+$/ ){
+    if ( $userspec =~ /^\d+$/ ) {
       $suspend_user = qsearchs( 'svc_acct', { 'svcnum' => $userspec } );
-    }elsif ($userspec =~ /^\S+\@\S+$/){
+    } elsif ( $userspec =~ /^\S+\@\S+$/ ) {
       my ($username,$domain) = split(/\@/, $userspec);
       for my $user (qsearch( 'svc_acct', { 'username' => $username } )){
         $suspend_user = $user if $userspec eq $user->email;
       }
-    }elsif ($userspec){
+    } elsif ($userspec) {
       $suspend_user = qsearchs( 'svc_acct', { 'username' => $userspec } );
     }
 
     @radius_groups = $suspend_user->radius_groups
       if $suspend_user;  
 
-  }else{
+  } else {
     $reasonnum = $reasontext = $reasontypenum = $reasontypetext = '';
   }
 
+  my $stdin_string = eval(qq("$stdin"));
+
+  $first = shell_quote $first;
+  $last = shell_quote $last;
+  $finger = shell_quote $finger;
+  $crypt_password = shell_quote $crypt_password;
+
+  my $command_string = eval(qq("$command"));
+
   $self->shellcommands_queue( $svc_acct->svcnum,
     user         => $self->option('user')||'root',
     host         => $self->machine,
-    command      => eval(qq("$command")),
-    stdin_string => eval(qq("$stdin")),
+    command      => $command_string,
+    stdin_string => $stdin_string,
   );
 }
 
@@ -308,16 +314,12 @@ sub _export_replace {
   }
   $new_finger =~ /^(.*)\s+(\S+)$/ or $new_finger =~ /^((.*))$/;
   ($new_first, $new_last ) = ( $1, $2 );
-  $new_first = shell_quote $new_first;
-  $new_last = shell_quote $new_last;
-  $new_finger = shell_quote $new_finger;
   $quoted_new__password = shell_quote $new__password; #old, wrong?
   $new_quoted_password = shell_quote $new__password; #new, better?
   $old_domain = $old->domain;
   $new_domain = $new->domain;
 
-  $new_crypt_password =
-    shell_quote( $new->crypt_password( $self->option('crypt') ) );
+  $new_crypt_password = $new->crypt_password( $self->option('crypt') );
 
   @old_radius_groups = $old->radius_groups;
   @new_radius_groups = $new->radius_groups;
@@ -349,11 +351,20 @@ sub _export_replace {
   return $error. ' ('. $self->exporttype. ' to '. $self->machine. ')'
     if $error;
 
+  my $stdin_string = eval(qq("$stdin"));
+
+  $new_first = shell_quote $new_first;
+  $new_last = shell_quote $new_last;
+  $new_finger = shell_quote $new_finger;
+  $new_crypt_password = shell_quote $crypt_password;
+
+  my $command_string = eval(qq("$command"));
+
   $self->shellcommands_queue( $new->svcnum,
     user         => $self->option('user')||'root',
     host         => $self->machine,
-    command      => eval(qq("$command")),
-    stdin_string => eval(qq("$stdin")),
+    command      => $command_string,
+    stdin_string => $stdin_string,
   );
 }