Be cautious about what can be unserialized

pear · Mar 13, 2022 · 6447c17 · 6447c17
1 parent d40fe6d
commit 6447c17
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 20 deletions.
diff --git a/include/bugs/pear-bug-accountrequest.php b/include/bugs/pear-bug-accountrequest.php
@@ -236,7 +236,11 @@ function confirmRequest($handle, $password, $name)
         if (!isset($user['registered'])) {
             return false;
         }
-        @$arr = unserialize($user['userinfo']);
+        try {
+            @$arr = unserialize($user['userinfo'], ['allowed_classes' => false]);
+        } catch (Exception $ex) {
+            $arr = false;
+        }
 
         include_once 'pear-database-note.php';
         note::removeAll($handle);

diff --git a/include/pear-database-user.php b/include/pear-database-user.php
@@ -46,7 +46,11 @@ static function activate($uid, $karmalevel = 'pear.dev')
         if (!isset($user['registered'])) {
             return false;
         }
-        @$arr = unserialize($user['userinfo']);
+        try {
+            @$arr = unserialize($user['userinfo'], ['allowed_classes' => false]);
+        } catch (Exception $ex) {
+            $arr = false;
+        }
 
         include_once 'pear-database-note.php';
         note::removeAll($uid);

diff --git a/include/pepr/pepr-ppvote.php b/include/pepr/pepr-ppvote.php
@@ -57,7 +57,14 @@ function get(&$dbh, $proposalId, $handle)
             return null;
         }
         $set = $res->fetchRow(DB_FETCHMODE_ASSOC);
-        $set['reviews'] = unserialize($set['reviews']);
+        try {
+            $unserialised = unserialize($set['reviews'], ['allowed_classes' => false]);
+            if ($unserialised !== false) {
+                $set['reviews'] = $unserialised;
+            }
+        } catch (Exception $ex) {
+            $set['reviews'] = array();
+        }
         $vote = new ppVote($set);
         return $vote;
     }
@@ -71,7 +78,10 @@ function &getAll(&$dbh, $proposalId)
         }
         $votes = array();
         while ($set = $res->fetchRow(DB_FETCHMODE_ASSOC)) {
-            $set['reviews'] = unserialize($set['reviews']);
+            $uReviews = unserialize($set['reviews'], ['allowed_classes' => false]);
+            if ($uReviews !== false) {
+                $set['reviews'] = $uReviews;
+            }
             $votes[$set['user_handle']] = new ppVote($set);
         }
         return $votes;

diff --git a/include/users/passwordmanage.php b/include/users/passwordmanage.php
@@ -55,7 +55,12 @@ function resetPassword($user, $pass1, $pass2)
     {
         require_once 'Damblan/Mailer.php';
         $errors = array();
-        $salt = md5(mt_rand(4,13) . $user . time() . $pass1);
+        $random_bytes = openssl_random_pseudo_bytes(16, $strong);
+        if ($random_bytes === false || $strong === false) {
+            $errors[] = "Could not generate a safe password token";
+            return $errors;
+        }
+        $salt = md5($rand_bytes);
         PEAR::staticPushErrorHandling(PEAR_ERROR_RETURN);
         $this->_dbh->query('DELETE FROM lostpassword WHERE handle=?', array($user));
         $e = $this->_dbh->query('INSERT INTO lostpassword
@@ -91,4 +96,4 @@ function resetPassword($user, $pass1, $pass2)
         }
         return $errors;
     }
-}
+}
diff --git a/pearweb.php b/pearweb.php
@@ -145,8 +145,13 @@ function initializeDatabase($answers)
             $oldversion = false;
         }
         if ($oldversion) {
-            $curdef = unserialize(file_get_contents($updir .
-              $answers['database'] . '-' . $oldversion . '.ser'));
+            $sFile = $updir . $answers['database'] . '-' . $oldversion . '.ser';
+            try {
+                $curdef = unserialize(file_get_contents($sFile), ['allowed_classes' => false]);
+            } catch (Exception $ex) {
+                $curdef = false;
+            }
+
             if (!is_array($curdef)) {
                 $this->_ui->outputData('invalid data returned from previous version');
             }
@@ -355,4 +360,4 @@ function setupHttpdconf($answers)
         $this->_ui->outputData('...done');
         return true;
     }
-}
+}
diff --git a/pearweb_election.php b/pearweb_election.php
@@ -138,8 +138,12 @@ function initializeDatabase($answers)
             $oldversion = false;
         }
         if ($oldversion) {
-            $curdef = unserialize(file_get_contents('@www-dir@/sql/.pearweb-upgrade/' .
-              $answers['database'] . '-' . $oldversion . '.ser'));
+            $sFile = '@www-dir@/sql/.pearweb-upgrade/' . $answers['database'] . '-' . $oldversion . '.ser';
+            try {
+                $curdef = unserialize(file_get_contents($sFile), ['allowed_classes' => false]);
+            } catch (Exception $ex) {
+                $curdef = false;
+            }
             if (!is_array($curdef)) {
                 $this->_ui->outputData('invalid data returned from previous version');
             }
@@ -166,4 +170,4 @@ function initializeDatabase($answers)
         }
         return true;
     }
-}
+}
diff --git a/pearweb_pepr.php b/pearweb_pepr.php
@@ -145,8 +145,12 @@ function initializeDatabase($answers)
             $oldversion = false;
         }
         if ($oldversion) {
-            $curdef = unserialize(file_get_contents('@www-dir@/sql/.pearweb-upgrade/' .
-              $answers['database'] . '-' . $oldversion . '.ser'));
+            try {
+                $sFile = '@www-dir@/sql/.pearweb-upgrade/' . $answers['database'] . '-' . $oldversion . '.ser';
+                $curdef = unserialize(file_get_contents($sFile), ['allowed_classes' => false]);
+            } catch (Exception $ex) {
+                $curdef = false;
+            }
             if (!is_array($curdef)) {
                 $this->_ui->outputData('invalid data returned from previous version');
             }
@@ -322,4 +326,4 @@ function setupHttpdconf($answers)
         $this->_ui->outputData('...done');
         return true;
     }
-}
+}
diff --git a/public_html/admin/index.php b/public_html/admin/index.php
@@ -267,7 +267,15 @@ function updateRejectReason(selectObj) {
         if (empty($requser['name']) || $requser['from_site'] == 'pecl') {
             break;
         }
-        list($purpose, $moreinfo) = @unserialize($requser['userinfo']);
+        try {
+            $uInfo = @unserialize($requser['userinfo'], ['allowed_classes' => false]);
+            if ($uInfo !== false) {
+                list($purpose, $moreinfo) = $uInfo;
+            }
+        } catch (Exception $ex) {
+            $purpose = 'n/a';
+            $moreinfo = 'n/a';
+        }
 
         $bb = new BorderBox('Account request from ' . $requser['name'] . ' &lt;' . $requser['email'] . '&gt;', "100%", '', 2, true);
         $bb->horizHeadRow("Requested username:", $requser['handle']);
@@ -511,8 +519,8 @@ function setCmdInput(mode)
             list($name, $note, $userinfo) = $data;
 
                 // Grab userinfo/request purpose
-            if (@unserialize($userinfo)) {
-                $userinfo = @unserialize($userinfo);
+            if (@unserialize($userinfo, ['allowed_classes' => false])) {
+                $userinfo = @unserialize($userinfo, ['allowed_classes' => false]);
                 $account_purpose = $userinfo[0];
             } else {
                 $account_purpose = $userinfo;

diff --git a/rollback.php b/rollback.php
@@ -74,7 +74,16 @@ protected function moveVotes()
             $comment  = "Original vote: {$row->value}\n";
             $comment .= "Conditional vote: " . ($row->is_conditional != 0)?'yes':'no' . "\n";
             $comment .= "Comment on vote: " . $row->comment . "\n\n";
-            $comment .= "Reviewed: " . implode(", ", unserialize($row->reviews));
+            $reviewed = "Reviewed: n/a";
+            try {
+                $uInfo = unserialize($row->reviews, ['allowed_classes' => false]);
+                if ($uInfo !== false) {
+                    $reviewed = "Reviewed: " . implode(", ", $uInfo);
+                }
+            } catch (Exception $ex) {
+                // do nothing
+            }
+            $comment .= $reviewed;
 
             $sql = sprintf(
                 $insert,

diff --git a/scripts/rollbackProposal.php b/scripts/rollbackProposal.php
@@ -74,7 +74,16 @@ protected function moveVotes()
             $comment  = "Original vote: {$row->value}\n";
             $comment .= "Conditional vote: " . (($row->is_conditional != 0)?'yes':'no') . "\n";
             $comment .= "Comment on vote: " . $row->comment . "\n";
-            $comment .= "Reviewed: " . implode(", ", unserialize($row->reviews));
+            $reviewed = "Reviewed: n/a";
+            try {
+                $uInfo = unserialize($row->reviews, ['allowed_classes' => false]);
+                if ($uInfo !== false) {
+                    $reviewed = "Reviewed: " . implode(", ", $uInfo);
+                }
+            } catch (Exception $ex) {
+                // do nothing
+            }
+            $comment .= $reviewed;
 
             $sql = sprintf(
                 $insert,