HEADS UP: Package maintainers – Check for call-time pass-by-reference usage!

phil.davis

For ease of reference, here is a sample way of doing pfSense version testing, which is already in a few packages:

// Check pfSense version
$pfs_version = substr(trim(file_get_contents("/etc/version")),0,3);
switch ($pfs_version) {
  case "1.2":
  case "2.0":
    define('PKG_BANDWIDTHD_BASE', '/usr/local/bandwidthd');
    define('PKG_BANDWIDTHD_RUNTIME_LIBRARY_ENV', '');
    break;
  case "2.1":
    define('PKG_BANDWIDTHD_BASE', '/usr/pbi/bandwidthd-' . php_uname("m") . '/bandwidthd');
    define('PKG_BANDWIDTHD_RUNTIME_LIBRARY_ENV', '');
    break;
  default:
    define('PKG_BANDWIDTHD_BASE', '/usr/pbi/bandwidthd-' . php_uname("m") . '/local/bandwidthd');
    define('PKG_BANDWIDTHD_RUNTIME_LIBRARY_ENV', 'LD_LIBRARY_PATH=/usr/pbi/bandwidthd-' . php_uname("m") . '/local/lib');
 }
// End: Check pfSense version

For the call-by-reference changes, the "2.1" case won't be needed - "default:" will catch 2.1.* 2.2.* and on into the future. The different do_input_validation calls will end up in the appropriate "case" sections.

jimp

For this purpose, a simpler test is sufficient:


     $pf_version=substr(trim(file_get_contents("/etc/version")),0,3);
     if ($pf_version < 2.1)
          $input_errors = eval('do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors); return $input_errors;');
     else
          do_input_validation($_POST, $reqdfields, $reqdfieldsn, $input_errors);

jimp

Note that I updated the example test for pre-2.1 versions. PHP 5.5 will produce an error for call-time pass-by-reference usage even if the code would never be encountered. So it had to be wrapped up in an eval to make sure that the behavior worked as desired.

jimp

I have fixed most of them except for ones I knew were being maintained by others.

The following will need to be fixed properly before the packages will work on 2.2:

Snort:

config/snort/snort_interfaces_suppress_edit.php:93:     do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/snort/snort_passlist_edit.php:115:       do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);

Suricata:

config/suricata/suricata_passlist_edit.php:117: do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/suricata/suricata_suppress_edit.php:91:  do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);

Dansguardian:

config/dansguardian/dansguardian.xml:380:               dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_antivirus_acl.xml:234:         dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_blacklist.xml:166:             dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_config.xml:309:                dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_content_acl.xml:202:           dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_file_acl.xml:242:              dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_groups.xml:453:                dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_header_acl.xml:222:            dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_ldap.xml:167:          dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_limits.xml:176:                dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_log.xml:249:           dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_phrase_acl.xml:265:            dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_pics_acl.xml:199:              dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_search_acl.xml:259:            dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_site_acl.xml:295:              dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_sync.xml:161:          dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_url_acl.xml:346:               dansguardian_validate_input($_POST, &$input_errors);
config/dansguardian/dansguardian_users_footer.template:9:               dansguardian_validate_input($_POST, &$input_errors);

HAproxy-devel:

config/haproxy-devel/haproxy_global.php:67:             //do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/haproxy-devel/haproxy_listeners_edit.php:147:    do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/haproxy-devel/haproxy_pool_edit.php:131: do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/haproxy-devel/haproxy_pool_edit.php:136:         do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/haproxy-devel/haproxy_pool_edit.php:140:                 do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);

Mailscanner:

config/mailscanner/mailscanner.xml:350:         mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_alerts.xml:153:          mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_antispam.xml:448:                mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_antivirus.xml:184:               mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_attachments.xml:215:             mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_content.xml:237:         mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_report.xml:527:          mailscanner_validate_input($_POST, &$input_errors);
config/mailscanner/mailscanner_sync.xml:154:            mailscanner_validate_input($_POST, &$input_errors);

pfBlocker:

config/pf-blocker/pfblocker.xml:244:            pfblocker_validate_input($_POST, &$input_errors);
config/pf-blocker/pfblocker_lists.xml:249:              pfblocker_validate_input($_POST, &$input_errors);
config/pf-blocker/pfblocker_sync.xml:141:               pfblocker_validate_input($_POST, &$input_errors);
config/pf-blocker/pfblocker_topspammers.xml:161:                pfblocker_validate_input($_POST, &$input_errors);

Postfix:

config/postfix/postfix.xml:357:         postfix_validate_input($_POST, &$input_errors);
config/postfix/postfix_acl.xml:224:             postfix_validate_input($_POST, &$input_errors);
config/postfix/postfix_antispam.xml:282:                postfix_validate_input($_POST, &$input_errors);
config/postfix/postfix_domains.xml:140:         postfix_validate_input($_POST, &$input_errors);
config/postfix/postfix_recipients.xml:195:              postfix_validate_input($_POST, &$input_errors);
config/postfix/postfix_sync.xml:196:            postfix_validate_input($_POST, &$input_errors);

Sarg:

config/sarg/sarg.xml:366:               sarg_validate_input($_POST, &$input_errors);
config/sarg/sarg_schedule.xml:219:              sarg_validate_input($_POST, &$input_errors);
config/sarg/sarg_sync.xml:141:          sarg_validate_input($_POST, &$input_errors);
config/sarg/sarg_users.xml:214:         sarg_validate_input($_POST, &$input_errors);

Squid-head (I'm not sure this is even active anywhere):

config/squid-head/squid.xml:201:                squid_before_form_general(&$pkg);
config/squid-head/squid.xml:204:                squid_validate_general($_POST, &$input_errors);
config/squid-head/squid_auth.xml:191:           squid_validate_auth($_POST, &$input_errors);
config/squid-head/squid_cache.xml:175:          squid_validate_cache($_POST, &$input_errors);
config/squid-head/squid_nac.xml:142:            squid_validate_nac($_POST, &$input_errors);
config/squid-head/squid_traffic.xml:174:                squid_validate_traffic($_POST, &$input_errors);
config/squid-head/squid_upstream.xml:128:               squid_validate_upstream($_POST, &$input_errors);

Squid 3.x and Squid 3-dev:

config/squid3/31/squid.xml:432:         squid_before_form_general(&$pkg);
config/squid3/31/squid.xml:438:         squid_validate_general($_POST, &$input_errors);
config/squid3/31/squid_auth.xml:247:            squid_validate_auth($_POST, &$input_errors);
config/squid3/31/squid_cache.xml:290:           squid_validate_cache($_POST, &$input_errors);
config/squid3/31/squid_nac.xml:181:             squid_validate_nac($_POST, &$input_errors);
config/squid3/31/squid_reverse.xml:349:         squid_before_form_general(&$pkg);
config/squid3/31/squid_reverse.xml:352:         squid_validate_reverse($_POST, &$input_errors);
config/squid3/31/squid_reverse_general.xml:241:         squid_before_form_general(&$pkg);
config/squid3/31/squid_reverse_general.xml:244:         squid_validate_reverse($_POST, &$input_errors);
config/squid3/31/squid_reverse_peer.xml:159:            squid_before_form_general(&$pkg);
config/squid3/31/squid_reverse_peer.xml:162:            squid_validate_reverse($_POST, &$input_errors);
config/squid3/31/squid_traffic.xml:198:         squid_validate_traffic($_POST, &$input_errors);
config/squid3/31/squid_upstream.xml:352:                squid_validate_upstream($_POST, &$input_errors);
config/squid3/33/squid.xml:558:         squid_before_form_general(&$pkg);
config/squid3/33/squid.xml:564:         squid_validate_general($_POST, &$input_errors);
config/squid3/33/squid_auth.xml:253:            squid_validate_auth($_POST, &$input_errors);
config/squid3/33/squid_cache.xml:315:           squid_validate_cache($_POST, &$input_errors);
config/squid3/33/squid_nac.xml:186:             squid_validate_nac($_POST, &$input_errors);
config/squid3/33/squid_reverse.xml:349:         squid_before_form_general(&$pkg);
config/squid3/33/squid_reverse.xml:352:         squid_validate_reverse($_POST, &$input_errors);
config/squid3/33/squid_reverse_general.xml:244:         squid_before_form_general(&$pkg);
config/squid3/33/squid_reverse_general.xml:247:         squid_validate_reverse($_POST, &$input_errors);
config/squid3/33/squid_reverse_peer.xml:159:            squid_before_form_general(&$pkg);
config/squid3/33/squid_reverse_peer.xml:162:            squid_validate_reverse($_POST, &$input_errors);
config/squid3/33/squid_traffic.xml:203:         squid_validate_traffic($_POST, &$input_errors);
config/squid3/33/squid_upstream.xml:356:                squid_validate_upstream($_POST, &$input_errors);
config/squid3/old/squid.xml:318:                squid_before_form_general(&$pkg);
config/squid3/old/squid.xml:324:                squid_validate_general($_POST, &$input_errors);
config/squid3/old/squid_auth.xml:223:           squid_validate_auth($_POST, &$input_errors);
config/squid3/old/squid_cache.xml:217:          squid_validate_cache($_POST, &$input_errors);
config/squid3/old/squid_nac.xml:138:            squid_validate_nac($_POST, &$input_errors);
config/squid3/old/squid_traffic.xml:172:                squid_validate_traffic($_POST, &$input_errors);
config/squid3/old/squid_upstream.xml:128:               squid_validate_upstream($_POST, &$input_errors);

Squidguard-devel:

config/squidGuard-devel/squidguard.inc:109:    if ($submit === APPLY_BTN)  sg_check_config_data(&$input_errors);
config/squidGuard-devel/squidguard.inc:117:    squidguard_validate_acl($post, &$input_errors);
config/squidGuard-devel/squidguard.inc:137:        check_name_format($name, &$input_errors);
config/squidGuard-devel/squidguard.inc:151:        sg_check_src($sgx, &$input_errors);
config/squidGuard-devel/squidguard.inc:195:    if (!sg_check_redirect($post[F_RMOD], $post[F_REDIRECT], &$errmsg)) {
config/squidGuard-devel/squidguard.inc:213:        check_name_format($name, &$input_errors);
config/squidGuard-devel/squidguard.inc:249:    sg_check_time($sgx, &$input_errors);
config/squidGuard-devel/squidguard.inc:260:        check_name_format($name, &$input_errors);
config/squidGuard-devel/squidguard.inc:280:    sg_check_dest($sgx, &$input_errors);
config/squidGuard-devel/squidguard.inc:291:            check_name_format($name, &$input_errors);
config/squidGuard-devel/squidguard.inc:1304:    squidguard_adt_safesrch_add(&$res[F_ITEM]);
config/squidGuard-devel/squidguard.inc:1377:    $cont = squidguard_logdump(SQUIDGUARD_LOGDIR . '/squidGuard.log', &$lnoffset, $lncount, $reverse);
config/squidGuard-devel/squidguard.inc:1391:    $cont = squidguard_logdump(SQUIDGUARD_LOGDIR . SQUIDGUARD_CONFLOGFILE, &$lnoffset, $lncount, $reverse);
config/squidGuard-devel/squidguard.inc:1405:    $cont = squidguard_logdump(SQUIDGUARD_LOGDIR . '/' . SQUIDGUARD_LOGFILE, &$lnoffset, $lncount, $reverse);
config/squidGuard-devel/squidguard.xml:242:                squidguard_validate(&$_POST, &$input_errors);
config/squidGuard-devel/squidguard.xml:245:                squidguard_before_form(&$pkg);
config/squidGuard-devel/squidguard_acl.xml:227:                         squidguard_validate_acl(&$_POST, &$input_errors);
config/squidGuard-devel/squidguard_acl.xml:230:                         squidguard_before_form_acl(&$pkg);
config/squidGuard-devel/squidguard_configurator.inc:849:    if (!sg_check_config_data(&$error_res)) {
config/squidGuard-devel/squidguard_configurator.inc:1074:                    acl_remove_blacklist_items(&$acl[F_DESTINATIONNAME]);
config/squidGuard-devel/squidguard_configurator.inc:1075:                    acl_remove_blacklist_items(&$acl[F_OVERDESTINATIONNAME]);
config/squidGuard-devel/squidguard_configurator.inc:1131:            acl_remove_blacklist_items(&$def[F_DESTINATIONNAME]);
config/squidGuard-devel/squidguard_configurator.inc:1257:    if (!sg_check_redirect($redirect_mode, $rdr_info, &$errmsg)) {
config/squidGuard-devel/squidguard_configurator.inc:1330:            if (!check_name_format($tm_name, &$err_s))
config/squidGuard-devel/squidguard_configurator.inc:1337:            sg_check_time($tm, &$elog);
config/squidGuard-devel/squidguard_configurator.inc:1348:            if (!check_name_format($src_name, &$err_s))
config/squidGuard-devel/squidguard_configurator.inc:1365:            if (!check_name_format($dst_name, &$err_s))
config/squidGuard-devel/squidguard_configurator.inc:1371:            sg_check_dest($dst, &$elog);
config/squidGuard-devel/squidguard_configurator.inc:1399:            if (!check_name_format($rw_name, &$err_s))
config/squidGuard-devel/squidguard_configurator.inc:1755:    array_packitems(&$dm);
config/squidGuard-devel/squidguard_configurator.inc:1756:    array_packitems(&$ur);
config/squidGuard-devel/squidguard_configurator.inc:1768:    sg_check_redirect($sgx[F_RMOD], $sgx[F_REDIRECT], &$elog);
config/squidGuard-devel/squidguard_default.xml:137:                             squidguard_validate_acl(&$_POST, &$input_errors);
config/squidGuard-devel/squidguard_default.xml:140:                             squidguard_before_form_acl(&$pkg, false);
config/squidGuard-devel/squidguard_dest.xml:175:                                squidguard_before_form_dest(&$pkg);
config/squidGuard-devel/squidguard_dest.xml:178:                                squidguard_validate_destination($_POST, &$input_errors);
config/squidGuard-devel/squidguard_log.php:80:              $res = squidguard_logrep(squidguard_guidump( &$offset, 50, true));
config/squidGuard-devel/squidguard_log.php:83:              $res = squidguard_logrep(squidguard_filterdump( &$offset, 50, true));
config/squidGuard-devel/squidguard_log.php:87:              $res = squidguard_logrep(squidguard_blockdump( &$offset, 50, true));
config/squidGuard-devel/squidguard_rewr.xml:139:                squidguard_validate_rewrite($_POST, &$input_errors);
config/squidGuard-devel/squidguard_time.xml:139:                squidguard_validate_times(&$_POST, &$input_errors);

If you are the maintainer of one of those packages and need assistance, look at my first post in this thread, and at the other recently committed fixes for call-time pass-by-reference. If you're still having trouble, reply here.

Thanks!

bmeeks

@jimp:

I have fixed most of them except for ones I knew were being maintained by others.

The following will need to be fixed properly before the packages will work on 2.2:

Snort:
config/snort/snort_interfaces_suppress_edit.php:93:     do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/snort/snort_passlist_edit.php:115:       do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
Suricata:
config/suricata/suricata_passlist_edit.php:117: do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
config/suricata/suricata_suppress_edit.php:91:  do_input_validation($_POST, $reqdfields, $reqdfieldsn, &$input_errors);
…

If you are the maintainer of one of those packages and need assistance, look at my first post in this thread, and at the other recently committed fixes for call-time pass-by-reference. If you're still having trouble, reply here.

Thanks Jim for pinpointing the exact files and line numbers. I will fix this up in the Snort and Suricata packages as part of their next updates. Is the PBI environment on 2.2 static now? By that I mean are the conf file installation paths and the shared library issues resolved? The conf file paths seemed to be /usr/pbi/{package_name-arch}/local/etc in some of the early 2.2 tests I did. The 2.1 path omitted that extra local directory. If the extra local is going to remain, then packages like Snort and Suricata (and some others) that need to reference their configuration file paths directly will have to have three different locations depending on pfSense version (2.0.x, 2.1.x and 2.2).

Bill

jimp

@bmeeks:

Thanks Jim for pinpointing the exact files and line numbers. I will fix this up in the Snort and Suricata packages as part of their next updates. Is the PBI environment on 2.2 static now? By that I mean are the conf file installation paths and the shared library issues resolved? The conf file paths seemed to be /usr/pbi/{package_name-arch}/local/etc in some of the early 2.2 tests I did. The 2.1 path omitted that extra local directory. If the extra local is going to remain, then packages like Snort and Suricata (and some others) that need to reference their configuration file paths directly will have to have three different locations depending on pfSense version (2.0.x, 2.1.x and 2.2).

I'm not 100% sure on the config issue. The libraries and such should be OK now. I've tested several packages after Renato's last round of fixes and they've all worked when they were broken before. Granted they were simpler ones like client export, sudo, and nmap but they were all broken before as well, but work now.

bmeeks

@jimp:

I'm not 100% sure on the config issue. The libraries and such should be OK now. I've tested several packages after Renato's last round of fixes and they've all worked when they were broken before. Granted they were simpler ones like client export, sudo, and nmap but they were all broken before as well, but work now.

I just tested Suricata on a 2.2 install and it works now if I account for the configuration files being in /usr/pbi/{pkg_name-arch}/local/etc. So I can fix this by including an additional "if case" in the GUI code that defines the path to the conf files in Suricata and Snort.

So it looks like all the shared library issues are resolved. I am working now on fixing up Snort and Suricata to take care of the call-time pass-by-reference issue. I can accommodate this other 2.2 change in the same update.

Bill

salida

Can someone please confirm which packages are working , and which not ?

salida

Just tested squid 2.7.9_4 and squidguard 1.4_4 pkg v.1.9.6.

Squid is still not working displaying the call by reference error.
Squidguard seems ok but depends on squid to work.

Is squid fixed and i am the only one having still this error ?
or the problem persists ?

2.2-ALPHA (i386) built on Fri Jun 13 09:58:26 CDT 2014

athurdent

Sadly it seems that the maintainer of freeradius2 won't be able to do any more work on that package.

https://forum.pfsense.org/index.php?topic=43675.msg432223#msg432223

jimp

@athurdent:

Sadly it seems that the maintainer of freeradius2 won't be able to do any more work on that package.

https://forum.pfsense.org/index.php?topic=43675.msg432223#msg432223

That package does not appear to be affected by the problem for which this thread is intended. If it is broken in some other way on 2.2, post a new thread on the 2.2 board and someone may be able to look at the package if there enough detail on how to reproduce the issue.