Skip to content

Commit

Permalink
Add a helper function for unserialize(). Fix #15423
Browse files Browse the repository at this point in the history
For calls to unserialize() which do not check for errors, use the
helper function instead.
  • Loading branch information
marcos-ng committed Jun 4, 2024
1 parent 91628a2 commit 82e2245
Show file tree
Hide file tree
Showing 18 changed files with 63 additions and 39 deletions.
10 changes: 5 additions & 5 deletions src/etc/inc/captiveportal.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1551,8 +1551,8 @@ function captiveportal_free_dnrules($rulenos_start = 2000,
$cpruleslck = lock("captiveportalrulesdn", LOCK_EX);
}

$rules = unserialize(file_get_contents(
"{$g['vardb_path']}/captiveportaldn.rules"));
$rules = unserialize_data(file_get_contents(
"{$g['vardb_path']}/captiveportaldn.rules"), []);
$ridx = $rulenos_start;
while ($ridx < $rulenos_range_max) {
if (substr($rules[$ridx], 0, strlen($cpzone . '_')) == $cpzone . '_') {
Expand Down Expand Up @@ -1591,7 +1591,7 @@ function captiveportal_reserve_ruleno($ruleno) {

$cpruleslck = lock("captiveportalrulesdn", LOCK_EX);
if (file_exists("{$g['vardb_path']}/captiveportaldn.rules")) {
$rules = unserialize(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"));
$rules = unserialize_data(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"), array_pad(array(), 64500, false));
} else {
$rules = array_pad(array(), 64500, false);
}
Expand All @@ -1612,7 +1612,7 @@ function captiveportal_get_next_dn_ruleno($rule_type = 'default', $rulenos_start
$cpruleslck = lock("captiveportalrulesdn", LOCK_EX);
$ruleno = 0;
if (file_exists("{$g['vardb_path']}/captiveportaldn.rules")) {
$rules = unserialize(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"));
$rules = unserialize_data(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"), []);
$ridx = $rulenos_start;
while ($ridx < $rulenos_range_max) {
if (empty($rules[$ridx])) {
Expand Down Expand Up @@ -1646,7 +1646,7 @@ function captiveportal_free_dn_rulenos($rulenos) {

$cpruleslck = lock("captiveportalrulesdn", LOCK_EX);
if (file_exists("{$g['vardb_path']}/captiveportaldn.rules")) {
$rules = unserialize(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"));
$rules = unserialize_data(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"), []);
foreach ($rulenos as $ruleno) {
$rules[$ruleno] = false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/etc/inc/config.lib.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ function cleanup_backupcache($lock = false) {
function get_backups() {
global $g;
if (file_exists("{$g['cf_conf_path']}/backup/backup.cache")) {
$confvers = unserialize(file_get_contents("{$g['cf_conf_path']}/backup/backup.cache"));
$confvers = unserialize_data(file_get_contents("{$g['cf_conf_path']}/backup/backup.cache"), []);
$bakvers = array_keys($confvers);
$toreturn = array();
sort($bakvers);
Expand Down Expand Up @@ -1067,7 +1067,7 @@ function backup_config() {
copy(g_get('cf_conf_path') . '/config.xml', $bakfilename);

if (file_exists(g_get('cf_conf_path') . '/backup/backup.cache')) {
$backupcache = unserialize(file_get_contents(g_get('cf_conf_path') . '/backup/backup.cache'));
$backupcache = unserialize_data(file_get_contents(g_get('cf_conf_path') . '/backup/backup.cache'), []);
} else {
$backupcache = array();
}
Expand Down
2 changes: 1 addition & 1 deletion src/etc/inc/interfaces.inc
Original file line number Diff line number Diff line change
Expand Up @@ -5083,7 +5083,7 @@ EOD;
}

if (file_exists("{$g['tmp_path']}/dhcp6c_ifs")) {
$dhcp6crealifs_run = unserialize(file_get_contents("{$g['tmp_path']}/dhcp6c_ifs"));
$dhcp6crealifs_run = unserialize_data(file_get_contents("{$g['tmp_path']}/dhcp6c_ifs"), []);
} else {
$dhcp6crealifs_run = array();
}
Expand Down
6 changes: 3 additions & 3 deletions src/etc/inc/system.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1576,15 +1576,15 @@ function delete_static_route($id, $delete = false) {
}

if (file_exists("{$g['tmp_path']}/.system_routes.apply")) {
$toapplylist = unserialize(file_get_contents("{$g['tmp_path']}/.system_routes.apply"));
$toapplylist = unserialize_data(file_get_contents("{$g['tmp_path']}/.system_routes.apply"), []);
} else {
$toapplylist = array();
}

if (file_exists("{$g['tmp_path']}/staticroute_{$id}") &&
file_exists("{$g['tmp_path']}/staticroute_{$id}_gw")) {
$delete_targets = unserialize(file_get_contents("{$g['tmp_path']}/staticroute_{$id}"));
$delgw = lookup_gateway_ip_by_name(unserialize(file_get_contents("{$g['tmp_path']}/staticroute_{$id}_gw")));
$delete_targets = unserialize_data(file_get_contents("{$g['tmp_path']}/staticroute_{$id}"), []);
$delgw = lookup_gateway_ip_by_name(unserialize_data(file_get_contents("{$g['tmp_path']}/staticroute_{$id}_gw")));
if (count($delete_targets)) {
foreach ($delete_targets as $dts) {
if (is_subnetv4($dts)) {
Expand Down
24 changes: 24 additions & 0 deletions src/etc/inc/util.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4575,4 +4575,28 @@ function compare_files($f1, $f2) {
return (hash_file('sha256', $f1) == hash_file('sha256', $f2));
}

/**
* Helper function for unserialize() with error handling.
*
* @param ?string $path Data string to unserialize
* @param mixed $default Value to return in case of failure
* @param ?array $options Options to pass to unserialize()
*
* @return mixed $data The unserialized data
*/
function unserialize_data(?string $path, mixed $default = null, ?array $options = []):mixed {
if (empty($path) || !isset($options)) {
return $default;
}

$data = @unserialize($path, $options);

// check if the string was not unserialized
if (($data === false) && ($data == serialize(false))) {
return $default;
}

return $data;
}

?>
8 changes: 4 additions & 4 deletions src/etc/rc.carpmaster
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ if (!empty(config_get_path('captiveportal')) &&

if (is_array($resp) || !empty($resp)) { // $resp will be an array only if the communication was successful
// Contains array of connected users (will be stored in SQLite DB)
$connected_users = unserialize(base64_decode($resp['connected_users']));
$connected_users = unserialize_data(base64_decode($resp['connected_users']), []);
// Contains array of active vouchers (will be stored in active vouchers db)
$active_vouchers = unserialize(base64_decode($resp['active_vouchers']));
$active_vouchers = unserialize_data(base64_decode($resp['active_vouchers']), []);
// Contain bitmask of both in use and expired vouchers (will be stored in "used vouchers" db)
$expired_vouchers = unserialize(base64_decode($resp['expired_vouchers']));
$expired_vouchers = unserialize_data(base64_decode($resp['expired_vouchers']), []);
// Contains array of usedmacs (will be stored in usedmacs db)
$usedmacs = unserialize(base64_decode($resp['usedmacs']));
$usedmacs = unserialize_data(base64_decode($resp['usedmacs']), []);

$cpdb = captiveportal_read_db();
$unsetindexes = array_column($cpdb, 5);
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/pfSense/include/www/alias-utils.inc
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ function saveAlias($post, $id) {
$srid++;
}
if ($reload_static_route && file_exists($g['tmp_path'] . '/.system_routes.apply')) {
$toapplylist = unserialize(file_get_contents($g['tmp_path'] . '/.system_routes.apply'));
$toapplylist = unserialize_data(file_get_contents($g['tmp_path'] . '/.system_routes.apply'), []);
foreach ($toapplylist as $toapply) {
mwexec("{$toapply}");
}
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/pfSense/include/www/backup.inc
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ function listBackupsJSON() {

cleanup_backupcache(false);

$raw = unserialize(file_get_contents(g_get('cf_conf_path') . "/backup/backup.cache"));
$raw = unserialize_data(file_get_contents(g_get('cf_conf_path') . "/backup/backup.cache"), []);

$backups = array();
foreach($raw as $key => $value) {
Expand Down
4 changes: 2 additions & 2 deletions src/usr/local/pfSense/include/www/firewall_virtual_ip.inc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ function saveVIP($post, $json = false) {
}

if (file_exists("{$g['tmp_path']}/.firewall_virtual_ip.apply")) {
$toapplylist = unserialize(file_get_contents("{$g['tmp_path']}/.firewall_virtual_ip.apply"));
$toapplylist = unserialize_data(file_get_contents("{$g['tmp_path']}/.firewall_virtual_ip.apply"), []);
} else {
$toapplylist = array();
}
Expand Down Expand Up @@ -293,7 +293,7 @@ function applyVIP() {

$check_carp = false;
if (file_exists("{$g['tmp_path']}/.firewall_virtual_ip.apply")) {
$toapplylist = unserialize(file_get_contents("{$g['tmp_path']}/.firewall_virtual_ip.apply"));
$toapplylist = unserialize_data(file_get_contents("{$g['tmp_path']}/.firewall_virtual_ip.apply"), []);
foreach ($toapplylist as $vid => $ovip) {
if (!empty($ovip)) {
interface_vip_bring_down($ovip);
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/sbin/gmirror_status_check.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
// Check for gmirror.status
if (file_exists($status_file)) {
// If it exists, read status in
$previous_mirror_status = unserialize(file_get_contents($status_file));
$previous_mirror_status = unserialize_data(file_get_contents($status_file), []);
$previous_mirror_list = array_keys($previous_mirror_status);
sort($previous_mirror_list);
if (count($previous_mirror_status) > 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/www/diag_confbak.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
}
}

$confvers = unserialize(file_get_contents(g_get('cf_conf_path') . '/backup/backup.cache'));
$confvers = unserialize_data(file_get_contents(g_get('cf_conf_path') . '/backup/backup.cache'), []);

if ($_POST['newver'] != "") {
if (config_restore(g_get('conf_path') . '/backup/config-' . $_POST['newver'] . '.xml') == 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/usr/local/www/interfaces.php
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ interface_wireless_clone($wlanif, $wancfg);

$vlan_redo = [];
if (file_exists(g_get('tmp_path') . '/.interfaces.apply')) {
$toapplylist = unserialize(file_get_contents(g_get('tmp_path') . '/.interfaces.apply'));
$toapplylist = unserialize_data(file_get_contents(g_get('tmp_path') . '/.interfaces.apply'), []);
foreach ($toapplylist as $ifapply => $ifcfgo) {
$realif = get_real_interface($ifapply);
$ifmtu = get_interface_mtu($realif);
Expand Down Expand Up @@ -1718,7 +1718,7 @@ interface_dhcpv6_configure($if, $wancfg, true);
}

if (file_exists(g_get('tmp_path') . '/.interfaces.apply')) {
$toapplylist = unserialize(file_get_contents(g_get('tmp_path') . '/.interfaces.apply'));
$toapplylist = unserialize_data(file_get_contents(g_get('tmp_path') . '/.interfaces.apply'), []);
} else {
$toapplylist = [];
}
Expand Down
8 changes: 4 additions & 4 deletions src/usr/local/www/services_captiveportal_hasync.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@
}
} else {
// Contains array of connected users (will be stored in SQLite DB)
$connected_users = unserialize(base64_decode($resp['connected_users']));
$connected_users = unserialize_data(base64_decode($resp['connected_users']), []);
// Contains array of active vouchers (will be stored in active vouchers db)
$active_vouchers = unserialize(base64_decode($resp['active_vouchers']));
$active_vouchers = unserialize_data(base64_decode($resp['active_vouchers']), []);
// Contain bitmask of both in use and expired vouchers (will be stored in "used vouchers" db)
$expired_vouchers = unserialize(base64_decode($resp['expired_vouchers']));
$expired_vouchers = unserialize_data(base64_decode($resp['expired_vouchers']), []);
// Contains array of usedmacs (will be stored in usedmacs db)
$usedmacs = unserialize(base64_decode($resp['usedmacs']));
$usedmacs = unserialize_data(base64_decode($resp['usedmacs']), []);

foreach ($connected_users as $user) {
$pipeno = captiveportal_get_next_dn_ruleno('auth');
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/www/services_pppoe.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

if ($_POST['apply']) {
if (file_exists("{$g['tmp_path']}/.vpn_pppoe.apply")) {
$toapplylist = unserialize(file_get_contents("{$g['tmp_path']}/.vpn_pppoe.apply"));
$toapplylist = unserialize_data(file_get_contents("{$g['tmp_path']}/.vpn_pppoe.apply"), []);
foreach ($toapplylist as $pppoeid) {
if (!is_numeric($pppoeid)) {
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/www/services_pppoe_edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ function vpn_pppoe_get_id() {
}

if (file_exists("{$g['tmp_path']}/.vpn_pppoe.apply")) {
$toapplylist = unserialize(file_get_contents("{$g['tmp_path']}/.vpn_pppoe.apply"));
$toapplylist = unserialize_data(file_get_contents("{$g['tmp_path']}/.vpn_pppoe.apply"), []);
} else {
$toapplylist = array();
}
Expand Down
2 changes: 1 addition & 1 deletion src/usr/local/www/system_routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

$routes_apply_file = g_get('tmp_path') . '/.system_routes.apply';
if (file_exists($routes_apply_file)) {
$toapplylist = unserialize(file_get_contents($routes_apply_file));
$toapplylist = unserialize_data(file_get_contents($routes_apply_file), []);
foreach ($toapplylist as $toapply) {
mwexec($toapply);
}
Expand Down
6 changes: 3 additions & 3 deletions src/usr/local/www/system_routes_edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@
if (!empty($oroute)) {
$staticroute_file = g_get('tmp_path') . '/staticroute_' . $id;
if (file_exists($staticroute_file)) {
$old_targets = unserialize(file_get_contents($staticroute_file));
$old_targets = unserialize_data(file_get_contents($staticroute_file), []);
}
$staticroute_gw_file = $staticroute_file . '_gw';
if (file_exists($staticroute_gw_file)) {
$old_gateway = unserialize(file_get_contents($staticroute_gw_file));
$old_gateway = unserialize_data(file_get_contents($staticroute_gw_file), []);
}
}

Expand Down Expand Up @@ -178,7 +178,7 @@

$routes_apply_file = g_get('tmp_path') . '/.system_routes.apply';
if (file_exists($routes_apply_file)) {
$toapplylist = unserialize(file_get_contents($routes_apply_file));
$toapplylist = unserialize_data(file_get_contents($routes_apply_file), []);
} else {
$toapplylist = array();
}
Expand Down
12 changes: 6 additions & 6 deletions src/usr/local/www/xmlrpc.php
Original file line number Diff line number Diff line change
Expand Up @@ -872,15 +872,15 @@ public function captive_portal_sync($arguments, $timeout) {

return $returndata;
} elseif ($arguments['op'] === 'connect_user') {
$user = unserialize(base64_decode($arguments['user']));
$user = unserialize_data(base64_decode($arguments['user']), []);
$user['attributes']['allow_time'] = $user['allow_time'];

// pipeno might be different between primary and secondary
$pipeno = captiveportal_get_next_dn_ruleno('auth');
return portal_allow($user['clientip'], $user['clientmac'], $user['username'], $user['password'], null,
$user['attributes'], $pipeno, $user['authmethod'], $user['context'], $user['sessionid']);
} elseif ($arguments['op'] === 'disconnect_user') {
$session = unserialize(base64_decode($arguments['session']));
$session = unserialize_data(base64_decode($arguments['session']), []);
/* read database again, as pipeno might be different between primary & secondary */
$sessionid = SQLite3::escapeString($session['sessionid']);
$local_dbentry = captiveportal_read_db("WHERE sessionid = '{$sessionid}'");
Expand All @@ -891,15 +891,15 @@ public function captive_portal_sync($arguments, $timeout) {
return false;
}
} elseif ($arguments['op'] === 'remove_entries') {
$entries = unserialize(base64_decode($arguments['entries']));
$entries = unserialize_data(base64_decode($arguments['entries']), []);

return captiveportal_remove_entries($entries, true);
} elseif ($arguments['op'] === 'disconnect_all') {
$arguments = unserialize(base64_decode($arguments['arguments']));
$arguments = unserialize_data(base64_decode($arguments['arguments']), []);

return captiveportal_disconnect_all($arguments['term_cause'], $arguments['logout_reason'], true);
} elseif ($arguments['op'] === 'write_vouchers') {
$arguments = unserialize(base64_decode($arguments['arguments']));
$arguments = unserialize_data(base64_decode($arguments['arguments']), []);

if (is_array($arguments['active_and_used_vouchers_bitmasks'])) {
foreach ($arguments['active_and_used_vouchers_bitmasks'] as $roll => $used) {
Expand All @@ -917,7 +917,7 @@ public function captive_portal_sync($arguments, $timeout) {
}
return true;
} elseif ($arguments['op'] === 'write_usedmacs') {
$arguments = unserialize(base64_decode($arguments['arguments']));
$arguments = unserialize_data(base64_decode($arguments['arguments']), []);

captiveportal_write_usedmacs_db($arguments['usedmacs']);
return true;
Expand Down

0 comments on commit 82e2245

Please sign in to comment.