#
# Security fix for command injection issue (#1295)
# Licensed under the GPLv2
# Olivier Bilodeau <obilodeau@inverse.ca>
#
# old_revision [86db3de9e3e252bfe03b944c1921d4e595774e42]
#
# patch "pf/html/admin/common.php"
#  from [4f9a43dd976e0fa97e1a2b7995ed784be5e146ab]
#    to [e382c8c4dc84a71c8ff5683c5cd8ec34bc1cbb2c]
# 
# patch "pf/html/admin/person/edit.php"
#  from [63382b47e7db287650c74f9cc1593accd194bb0a]
#    to [365dfb0f0b01c55768b50484c40a06911a801f3e]
# 
# patch "pf/html/captive-portal/guest-selfregistration.cgi"
#  from [ac9721cbae992680c1f7b6781016d8d9984c6767]
#    to [e3645e6533120c8018610a41203fd0b765d2f9b9]
# 
# patch "pf/html/captive-portal/mobile-confirmation.cgi"
#  from [9949e3b429d49bb652cb6c579b42a3e56426f4a1]
#    to [d7e3eb2ee5c10c5265360b90f194ef8d69acb1b4]
# 
# patch "pf/lib/pf/web/guest.pm"
#  from [65f5bceace1ca913b7846a304f72db80858ec008]
#    to [6e48b4451ad79308568d3856beea063bf96c715d]
#
============================================================
--- pf/html/admin/common.php	4f9a43dd976e0fa97e1a2b7995ed784be5e146ab
+++ pf/html/admin/common.php	e382c8c4dc84a71c8ff5683c5cd8ec34bc1cbb2c
@@ -467,7 +467,7 @@ if($sajax){
     if($this->rows[$item+1])
       $value=implode("\t", $this->rows[$item+1]);
 
-    print "<input type='hidden' name='original' value='$value'>";
+    print "<input type='hidden' name='original' value='". htmlentities($value, ENT_QUOTES) ."'>";
     print "<input type='hidden' name='commit' value='true'>";
     print "<td width='50'><div id='submit'><input type='submit' value='Submit'></div></td>\n";
     print "</form>";
@@ -560,8 +560,8 @@ if($sajax){
                print "<form action='/$current_top/$current_sub.php?filter=$filter&amp;sort=$sort&amp;direction=$direction&amp;page_num=$this->page_num&amp;per_page=$this->per_page&amp;action=$action&amp;item=$item' method='post'>";
                print "  <input type='hidden' name='action' value='delete'>\n";
                print "  <input type='hidden' name='commit' value='true'>\n";
-               print "  <input type='hidden' name='original' value='".implode("\t", $this->rows[$i])."'>\n";
-               print "  <input class=\"button\" type='image' src='/images/delete.png' align=bottom title='Delete this record' onClick=\"return confirm('Are you sure you want to delete ".$this->rows[$i][$this->key]."?');\">\n";
+               print "  <input type='hidden' name='original' value='".htmlentities(implode("\t", $this->rows[$i]), ENT_QUOTES)."'>\n";
+               print "  <input class=\"button\" type='image' src='/images/delete.png' align=bottom title='Delete this record' onClick=\"return confirm('Are you sure you want to delete ".htmlentities($this->rows[$i][$this->key], ENT_QUOTES)."?');\">\n";
                print "  </form>";
              }
              print "</td>\n";
@@ -578,6 +578,9 @@ if($sajax){
 
            $a=-1;
            foreach($this->rows[$i] as $cell){
+             # XSS prevention
+             $cell = htmlentities($cell, ENT_QUOTES);
+
              $key=$this->headers[++$a];
 
              if($key == $this->key){
============================================================
--- pf/html/admin/person/edit.php	63382b47e7db287650c74f9cc1593accd194bb0a
+++ pf/html/admin/person/edit.php	365dfb0f0b01c55768b50484c40a06911a801f3e
@@ -81,11 +81,15 @@
     $pretty_key = pretty_header("$current_top-view", $key);
     switch($key) {
     case 'notes':
-      print "<tr><td></td><td>$pretty_key:</td><td></td></tr><tr><td colspan='3'><textarea name='$key' rows='5'>$val</textarea></td></tr>";
+      print "<tr><td></td><td>$pretty_key:</td><td></td></tr><tr><td colspan='3'><textarea name='$key' rows='5'>" 
+            . htmlentities($val) . 
+            "</textarea></td></tr>";
       break;
 
     default:
-      print "<tr><td></td><td>$pretty_key:</td><td><input type='text' name='$key' value='$val'></td></tr>";
+      print "<tr><td></td><td>$pretty_key:</td><td><input type='text' name='$key' value='"
+            . htmlentities($val, ENT_QUOTES) . 
+            "'></td></tr>";
     }
   }
   print "<tr><td colspan=3 align=right><input type='submit' value='Edit ".ucfirst($current_top)."'></td></tr>";
============================================================
--- pf/html/captive-portal/guest-selfregistration.cgi	ac9721cbae992680c1f7b6781016d8d9984c6767
+++ pf/html/captive-portal/guest-selfregistration.cgi	e3645e6533120c8018610a41203fd0b765d2f9b9
@@ -20,6 +20,7 @@ use pf::node;
 use pf::sms_activation;
 use pf::iplog;
 use pf::node;
+use pf::person qw(person_modify);
 use pf::util;
 use pf::violation;
 use pf::web;
@@ -66,16 +67,15 @@ if (defined($params{'mode'}) && $params{
       # User chose to register by email
       $logger->info("Registering guest by email");
 
-      # Adding person (using edit in case person already exists)
-      my $person_add_cmd = "$bin_dir/pfcmd 'person edit \""
-        . $session->param("login")."\" "
-        . "firstname=\"" . $session->param("firstname") . "\","
-        . "lastname=\"" . $session->param("lastname") . "\","
-        . "email=\"" . $session->param("email") . "\","
-        . "telephone=\"" . $session->param("phone") . "\","
-        . "notes=\"email activation\"'";
-      $logger->info("Registering guest person with command: $person_add_cmd");
-      pf_run("$person_add_cmd");
+      # Login successful, adding person (using modify in case person already exists)
+      person_modify($session->param("login"), (
+          'firstname' => $session->param("firstname"),
+          'lastname' => $session->param("lastname"),
+          'email' => $session->param("email"),
+          'telephone' => $session->param("phone"),
+          'notes' => 'email activation',
+      ));
+      $logger->info("Adding guest person " . $session->param("login"));
 
       # grab additional info about the node
       $info{'pid'} = $session->param("login");
============================================================
--- pf/html/captive-portal/mobile-confirmation.cgi	9949e3b429d49bb652cb6c579b42a3e56426f4a1
+++ pf/html/captive-portal/mobile-confirmation.cgi	d7e3eb2ee5c10c5265360b90f194ef8d69acb1b4
@@ -20,6 +20,7 @@ use pf::node;
 use pf::config;
 use pf::iplog;
 use pf::node;
+use pf::person qw(person_modify);
 use pf::util;
 use pf::violation;
 use pf::web;
@@ -100,16 +101,15 @@ if ($cgi->param("pin")) { # && $session-
       return(0);
     }
 
-    # Adding person (using edit in case person already exists)
-    my $person_add_cmd = "$bin_dir/pfcmd 'person edit \""
-      . $session->param("login")."\" "
-      . "firstname=\"" . $session->param("final_user_first_name") . "\","
-      . "lastname=\"" . $session->param("final_user_name") . "\","
-      . "email=\"" . $session->param("final_user_email") . "\","
-      . "telephone=\"" . $session->param("phone") . "\","
-      . "notes=\"sms confirmation\"'";
-    $logger->info("Registering guest person with command: $person_add_cmd");
-    pf_run("$person_add_cmd");
+    # Login successful, adding person (using modify in case person already exists)
+    $logger->info("Adding guest person $pid");
+    person_modify($pid, (
+        'firstname' => $session->param("firstname"),
+        'lastname' => $session->param("lastname"),
+        'email' => $session->param("email"),
+        'telephone' => $session->param("phone"),
+        'notes' => 'sms confirmation',
+    ));
 
     # Setting access timeout
     my @unregdate = localtime( time + normalize_time($pf::web::guest::DEFAULT_REGISTRATION_DURATION) );
============================================================
--- pf/lib/pf/web/guest.pm	65f5bceace1ca913b7846a304f72db80858ec008
+++ pf/lib/pf/web/guest.pm	6e48b4451ad79308568d3856beea063bf96c715d
@@ -577,18 +577,16 @@ sub preregister {
     bindtextdomain( "packetfence", "$conf_dir/locale" );
     textdomain("packetfence");
 
-    # Login successful, adding person (using edit in case person already exists)
-    my $person_add_cmd = "$bin_dir/pfcmd 'person edit \""
-        . $session->param("email")."\" "
-        . "firstname=\"" . $session->param("firstname") . "\","
-        . "lastname=\"" . $session->param("lastname") . "\","
-        . "email=\"" . $session->param("email") . "\","
-        . "telephone=\"" . $session->param("phone") . "\","
-        . "notes=\"".sprintf(i18n("Expected on %s"), $session->param("arrival_date")) . "\","
-        . "sponsor=\"".$session->param("login")."\"'"
-    ;
-    $logger->info("Adding guest person with command: $person_add_cmd");
-    pf_run("$person_add_cmd");
+    # Login successful, adding person (using modify in case person already exists)
+    person_modify($session->param("email"), (
+        'firstname' => $session->param("firstname"),
+        'lastname' => $session->param("lastname"),
+        'email' => $session->param("email"),
+        'telephone' => $session->param("phone"),
+        'notes' => sprintf(i18n("Expected on %s"), $session->param("arrival_date")),
+        'sponsor' => $session->param("login")
+    ));
+    $logger->info("Adding guest person " . $session->param("email"));
 
     # expiration is arrival date + access duration + a tolerance window of 24 hrs
     my $expiration = POSIX::strftime("%Y-%m-%d %H:%M:%S",