From 90ea643fa8c32b94f6f5ed636730d87eb31bba6b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Roland=20H=C3=A4der?= Date: Tue, 28 Jun 2011 09:00:38 +0000 Subject: [PATCH] More EL code, security for $_POST elements rewritten (simplified): - More usage of EL code - Removed double secureString() call - Non-array elements in $_POST are now also secured in inc/libs/security_functions.php - Renamed more array elements for better naming consistancy - TODOs.txt updated --- DOCS/TODOs.txt | 5 ++--- inc/libs/security_functions.php | 12 ++++++++++++ inc/libs/sponsor_functions.php | 8 ++++---- inc/libs/theme_functions.php | 4 ++-- inc/modules/admin.php | 4 ++-- inc/modules/admin/what-config_points.php | 11 +++++------ inc/modules/admin/what-del_sponsor.php | 2 +- inc/modules/admin/what-edit_sponsor.php | 4 ++-- inc/modules/admin/what-lock_sponsor.php | 2 +- inc/modules/admin/what-user_contct.php | 2 +- inc/modules/guest/what-sponsor_reg.php | 2 +- inc/modules/member/what-beg2.php | 11 +++-------- inc/modules/member/what-holiday.php | 2 +- inc/modules/member/what-transfer.php | 5 ++--- inc/monthly/monthly_beg.php | 4 ++-- inc/request-functions.php | 16 ++++++++-------- inc/session-functions.php | 3 ++- inc/stylesheet.php | 7 +++++-- .../de/emails/admin/admin_transfer_points.tpl | 2 +- templates/de/emails/member/member_beg.tpl | 2 +- .../emails/member/member_transfer_recipient.tpl | 2 +- .../de/emails/member/member_transfer_sender.tpl | 2 +- templates/de/html/member/member_list_beg_row.tpl | 2 +- 23 files changed, 61 insertions(+), 53 deletions(-) diff --git a/DOCS/TODOs.txt b/DOCS/TODOs.txt index bb04566458..8102d8aca6 100644 --- a/DOCS/TODOs.txt +++ b/DOCS/TODOs.txt @@ -123,7 +123,7 @@ ./inc/modules/admin/what-config_admins.php:108: // @TODO Rewrite this to a filter ./inc/modules/admin/what-config_admins.php:136: // @TODO Rewrite this to filter 'run_sqls' ./inc/modules/admin/what-config_mods.php:55: // @TODO This can be moved into mysql-function.php, see checkModulePermissions() function -./inc/modules/admin/what-config_points.php:111: // @TODO Rewrite this to a filter +./inc/modules/admin/what-config_points.php:110: // @TODO Rewrite this to a filter ./inc/modules/admin/what-config_rallye_prices.php:195: // @TODO Rewrite these two constants ./inc/modules/admin/what-config_register.php:75: // @TODO Move this HTML code into a template ./inc/modules/admin/what-del_email.php:61: // @TODO Unused: cat_id, payment_id @@ -164,7 +164,6 @@ ./inc/modules/guest/what-sponsor_reg.php:287: // @TODO Maybe a default referal id? ./inc/modules/guest/what-stats.php:100: // @TODO This can be somehow rewritten ./inc/modules/guest/what-stats.php:74:// @TODO This can be rewritten in a dynamic include -./inc/modules/member/what-beg2.php:87: // @TODO points->beg_points ./inc/modules/member/what-beg.php:54:// @TODO Can't this be moved into EL? ./inc/modules/member/what-beg.php:63:// @TODO No more needed? define('__BEG_UID_TIMEOUT', createFancyTime(getBegUseridTimeout())); ./inc/modules/member/what-bonus.php:55: // @TODO Rewrite this to a filter @@ -176,7 +175,7 @@ ./inc/modules/member/what-refback.php:124: // @TODO UNUSED: $refRow['status'] = translateUserStatus($refRow['status']); ./inc/modules/member/what-reflinks.php:52:// @TODO Move this into a filter ./inc/modules/member/what-transfer.php:134: // @TODO Rewrite this to a filter -./inc/modules/member/what-transfer.php:224: // @TODO Try to rewrite his to $content = SQL_FETCHARRAY(), see some lines above for two different queries +./inc/modules/member/what-transfer.php:223: // @TODO Try to rewrite his to $content = SQL_FETCHARRAY(), see some lines above for two different queries ./inc/modules/member/what-transfer.php:96: // @TODO Rewrite this to a filter ./inc/modules/member/what-unconfirmed.php:142: // @TODO Try to rewrite this to $content = SQL_FETCHARRAY() ./inc/modules/member/what-unconfirmed.php:207: // @TODO This 'userid' cannot be saved because of encapsulated EL code diff --git a/inc/libs/security_functions.php b/inc/libs/security_functions.php index 2b41ab968c..df3d5686b0 100644 --- a/inc/libs/security_functions.php +++ b/inc/libs/security_functions.php @@ -212,6 +212,18 @@ if (is_array($_GET)) { } // END - foreach } // END - if +// Secure also $_POST data (only simple, no replace) +if (is_array($_POST)) { + // Secure only simple data + foreach ($_POST as $seckey => $secvalue) { + // Is it an array? + if (!is_array($secvalue)) { + // Strip all other out + $_POST[$seckey] = secureString($_POST[$seckey]); + } // END - if + } // END - foreach +} // END - if + // Detect PHP caching detectPhpCaching(); diff --git a/inc/libs/sponsor_functions.php b/inc/libs/sponsor_functions.php index 6135e9108d..814a19b260 100644 --- a/inc/libs/sponsor_functions.php +++ b/inc/libs/sponsor_functions.php @@ -574,11 +574,11 @@ function doProcessSponsorFormRequest ($messageArray = array()) { // Prepare data for the email template $content['id'] = $id; $content['hash'] = $hash; - $content['email'] = secureString(postRequestParameter('email')); - $content['surname'] = secureString(postRequestParameter('surname')); - $content['family'] = secureString(postRequestParameter('family')); + $content['email'] = postRequestParameter('email'); + $content['surname'] = postRequestParameter('surname'); + $content['family'] = postRequestParameter('family'); $content['timestamp'] = generateDateTime(time(), 0); - $content['password'] = secureString(postRequestParameter('pass1')); + $content['password'] = postRequestParameter('pass1'); // Generate email and send it to the new sponsor $message = loadEmailTemplate('sponsor_confirm', $content, $id); diff --git a/inc/libs/theme_functions.php b/inc/libs/theme_functions.php index 976d45acc5..58d1fec2c2 100644 --- a/inc/libs/theme_functions.php +++ b/inc/libs/theme_functions.php @@ -248,9 +248,9 @@ function getActualTheme () { if ((isGetRequestParameterSet('theme')) && (isIncludeReadable($theme))) { // Set cookie from URL data setTheme(getRequestParameter('theme')); - } elseif (isIncludeReadable(sprintf("theme/%s/theme.php", secureString(postRequestParameter('theme'))))) { + } elseif (isIncludeReadable(sprintf("theme/%s/theme.php", postRequestParameter('theme')))) { // Set cookie from posted data - setTheme(secureString(postRequestParameter('theme'))); + setTheme(postRequestParameter('theme')); } // Set return value diff --git a/inc/modules/admin.php b/inc/modules/admin.php index 572888450f..f0c295eb5f 100644 --- a/inc/modules/admin.php +++ b/inc/modules/admin.php @@ -194,8 +194,8 @@ if (!isAdminRegistered()) { if ($valid === true) { // Prepare content first $content = array( - 'hash' => secureString(postRequestParameter('hash')), - 'login' => secureString(postRequestParameter('login')) + 'hash' => postRequestParameter('hash'), + 'login' => postRequestParameter('login') ); // Validation okay so display form for final password change diff --git a/inc/modules/admin/what-config_points.php b/inc/modules/admin/what-config_points.php index 3ef48975ae..a9cab39d96 100644 --- a/inc/modules/admin/what-config_points.php +++ b/inc/modules/admin/what-config_points.php @@ -77,10 +77,9 @@ if (isFormSent()) { break; case 'ref': - switch (getRequestParameter('do')) - { + switch (getRequestParameter('do')) { case 'add': - addSql("INSERT INTO `{?_MYSQL_PREFIX?}_refdepths` (`level`, `percents`) VALUES ('".postRequestParameter('level')."','".postRequestParameter('percents')."')"); + addSql("INSERT INTO `{?_MYSQL_PREFIX?}_refdepths` (`level`, `percents`) VALUES ('".bigintval(postRequestParameter('level'))."','".bigintval(postRequestParameter('percents'))."')"); break; case 'edit': // Change entries @@ -94,7 +93,7 @@ if (isFormSent()) { // Update entry SQL_QUERY_ESC("UPDATE `{?_MYSQL_PREFIX?}_refdepths` SET `level`=%s, `percents`=%s WHERE `id`=%s LIMIT 1", array(bigintval($value), convertCommaToDot(postRequestParameter('percents', $id)), $id), __FILE__, __LINE__); - } + } // END - foreach $message = '{--ADMIN_REFERAL_DEPTHS_SAVED--}'; break; @@ -102,10 +101,10 @@ if (isFormSent()) { foreach (postRequestParameter('id') as $id => $value) { SQL_QUERY_ESC("DELETE LOW_PRIORITY FROM `{?_MYSQL_PREFIX?}_refdepths` WHERE `id`=%s LIMIT 1", array(bigintval($id)), __FILE__, __LINE__); - } + } // END - foreach $message = '{--ADMIN_REFERAL_DEPTHS_DELETED--}'; break; - } + } // END - switch // Update cache file // @TODO Rewrite this to a filter diff --git a/inc/modules/admin/what-del_sponsor.php b/inc/modules/admin/what-del_sponsor.php index 25521563b1..14035c960a 100644 --- a/inc/modules/admin/what-del_sponsor.php +++ b/inc/modules/admin/what-del_sponsor.php @@ -54,7 +54,7 @@ if (isGetRequestParameterSet('id')) { $content = SQL_FETCHARRAY($result); // Prepare data for the template - $content['reason'] = secureString(postRequestParameter('reason')); + $content['reason'] = postRequestParameter('reason'); // Prepare message and send it away $message = loadEmailTemplate('del_sponsor', $content, bigintval(getRequestParameter('id'))); diff --git a/inc/modules/admin/what-edit_sponsor.php b/inc/modules/admin/what-edit_sponsor.php index 27b3978a21..d22020fcbe 100644 --- a/inc/modules/admin/what-edit_sponsor.php +++ b/inc/modules/admin/what-edit_sponsor.php @@ -83,7 +83,7 @@ LIMIT 1", // Remember points /reason for the template $content['points'] = $points; - $content['reason'] = secureString(postRequestParameter('reason')); + $content['reason'] = postRequestParameter('reason'); // Send email $message = loadEmailTemplate('sponsor_add_points', $content); @@ -106,7 +106,7 @@ LIMIT 1", // Remember points /reason for the template $content['points'] = $points; - $content['reason'] = secureString(postRequestParameter('reason')); + $content['reason'] = postRequestParameter('reason'); // Send email $message = loadEmailTemplate('sponsor_sub_points', $content); diff --git a/inc/modules/admin/what-lock_sponsor.php b/inc/modules/admin/what-lock_sponsor.php index 00b2e2560a..ed5d9ea4b5 100644 --- a/inc/modules/admin/what-lock_sponsor.php +++ b/inc/modules/admin/what-lock_sponsor.php @@ -55,7 +55,7 @@ if (isGetRequestParameterSet('id')) { if (($content['status'] == 'CONFIRMED') || ($content['status'] == 'LOCKED')) { // Transfer data to constants $content['id'] = bigintval(getRequestParameter('id')); - $content['reason'] = secureString(postRequestParameter('reason')); + $content['reason'] = postRequestParameter('reason'); if (isFormSent()) { // Create messages diff --git a/inc/modules/admin/what-user_contct.php b/inc/modules/admin/what-user_contct.php index ac51462b02..36c68abe8c 100644 --- a/inc/modules/admin/what-user_contct.php +++ b/inc/modules/admin/what-user_contct.php @@ -56,7 +56,7 @@ if ((isGetRequestParameterSet('userid')) && (bigintval(getRequestParameter('user // Shall we send the email? if (isFormSent()) { // Insert text - $content['text'] = trim(secureString(postRequestParameter('text'))); + $content['text'] = postRequestParameter('text'); // Load email template $message = loadEmailTemplate('member_contct', $content, getRequestParameter('userid')); diff --git a/inc/modules/guest/what-sponsor_reg.php b/inc/modules/guest/what-sponsor_reg.php index 3d0a93547e..9dd8629eb3 100644 --- a/inc/modules/guest/what-sponsor_reg.php +++ b/inc/modules/guest/what-sponsor_reg.php @@ -260,7 +260,7 @@ ORDER BY if (count($formErrors) > 0) { // Some found... :-( foreach (array('company','position','tax_ident','surname','family','street_nr1','street_nr2','country','zip','city','phone','fax','cell','email','url') as $entry) { - $content[$entry] = secureString(postRequestParameter($entry)); + $content[$entry] = postRequestParameter($entry); } // END - foreach // Init receive selection diff --git a/inc/modules/member/what-beg2.php b/inc/modules/member/what-beg2.php index 25b5bdce55..30ff574716 100644 --- a/inc/modules/member/what-beg2.php +++ b/inc/modules/member/what-beg2.php @@ -84,13 +84,8 @@ if (!SQL_HASZERONUMS($result)) { $count = 1; while ($content = SQL_FETCHARRAY($result)) { // Prepare data for the template - // @TODO points->beg_points - $content = array( - 'cnt' => $count, - 'userid' => $content['userid'], - 'points' => $content['beg_points'], - 'last_online' => generateDateTime($content['last_online'], 2), - ); + $content['cnt'] = $count; + $content['last_online'] = generateDateTime($content['last_online'], 2); // Load row template $OUT .= loadTemplate('member_list_beg_row', true, $content); @@ -99,7 +94,7 @@ if (!SQL_HASZERONUMS($result)) { $count++; } // END - while } else { - // No one is interested in our "active rallye" ! :-( + // No one is interested in our "begging rallye" ! :-( $OUT = loadTemplate('member_beg_404', true); } diff --git a/inc/modules/member/what-holiday.php b/inc/modules/member/what-holiday.php index 10c76eea90..46baf1cd64 100644 --- a/inc/modules/member/what-holiday.php +++ b/inc/modules/member/what-holiday.php @@ -139,7 +139,7 @@ LIMIT 1", $content['end_day'] = bigintval(postRequestParameter('end_day')); $content['end_month'] = $GLOBALS['month_descr'][postRequestParameter('end_month')]; $content['end_year'] = bigintval(postRequestParameter('end_year')); - $content['comments'] = secureString(postRequestParameter('comments')); + $content['comments'] = postRequestParameter('comments'); // Send mail to member $message = loadEmailTemplate('member_holiday_request', $content, getMemberId()); diff --git a/inc/modules/member/what-transfer.php b/inc/modules/member/what-transfer.php index e9e78c92b8..cf6fec76cf 100644 --- a/inc/modules/member/what-transfer.php +++ b/inc/modules/member/what-transfer.php @@ -142,9 +142,8 @@ switch ($mode) { } // END - if } // END - if - // Remember transfer reason and fancy date/time in constants - $content['reason'] = secureString(postRequestParameter('reason')); - $content['expires'] = '{%config,createFancyTime=transfer_age%}'; + // Remember transfer reason + $content['reason'] = postRequestParameter('reason'); // Generate tranafer id $content['trans_id'] = bigintval(generateRandomCode('10', mt_rand(0, 99999), getMemberId(), postRequestParameter('reason'))); diff --git a/inc/monthly/monthly_beg.php b/inc/monthly/monthly_beg.php index 232bee7b0e..c1effc065c 100644 --- a/inc/monthly/monthly_beg.php +++ b/inc/monthly/monthly_beg.php @@ -71,7 +71,7 @@ if ((getBegRanks() > 0) && (!isCssOutputMode())) { // SQL string to check for accounts $result_main = SQL_QUERY("SELECT - `userid`, `email`, `gender`, `surname`, `family`, `beg_points` AS `points` + `userid`, `email`, `gender`, `surname`, `family`, `beg_points` FROM `{?_MYSQL_PREFIX?}_user_data` WHERE @@ -86,7 +86,7 @@ LIMIT {?beg_ranks?}", __FILE__, __LINE__); // Load our winners... while ($content = SQL_FETCHARRAY($result_main)) { // Add points to user's account directly - addPointsDirectly('monthly_beg', $content['userid'], $content['points']); + addPointsDirectly('monthly_beg', $content['userid'], $content['beg_points']); // Load email template and email it away $message = loadEmailTemplate('member_beg', $content, bigintval($content['userid'])); diff --git a/inc/request-functions.php b/inc/request-functions.php index ee70e61402..799d4e38dd 100644 --- a/inc/request-functions.php +++ b/inc/request-functions.php @@ -52,15 +52,15 @@ function getRequestParameter ($element) { $value = null; // Is the element cached or there? - if (isset($GLOBALS['cache_request']['request_get'][$element])) { + if (isset($GLOBALS['cache_request']['get'][$element])) { // Then use the cache - $value = $GLOBALS['cache_request']['request_get'][$element]; + $value = $GLOBALS['cache_request']['get'][$element]; } elseif (isGetRequestParameterSet($element)) { // Then get it directly $value = SQL_ESCAPE($GLOBALS['raw_request']['get'][$element]); // Store it in cache - $GLOBALS['cache_request']['request_get'][$element] = $value; + $GLOBALS['cache_request']['get'][$element] = $value; } // END - if // Return value @@ -113,7 +113,7 @@ function setGetRequestParameter ($element, $value) { $GLOBALS['raw_request']['get'][$element] = $value; // Update cache - $GLOBALS['cache_request']['request_get'][$element] = $value; + $GLOBALS['cache_request']['get'][$element] = $value; } // Wrapper for elements in $_POST @@ -122,9 +122,9 @@ function postRequestParameter ($element, $subElement=null) { $value = null; // Is the element in cache? - if (isset($GLOBALS['cache_request']['request_post'][$element][$subElement])) { + if (isset($GLOBALS['cache_request']['post'][$element][$subElement])) { // Then use it - $value = $GLOBALS['cache_request']['request_post'][$element][$subElement]; + $value = $GLOBALS['cache_request']['post'][$element][$subElement]; } elseif (isPostRequestParameterSet($element)) { // Then use it $value = $GLOBALS['raw_request']['post'][$element]; @@ -139,7 +139,7 @@ function postRequestParameter ($element, $subElement=null) { } // Set it in cache - $GLOBALS['cache_request']['request_post'][$element][$subElement] = $value; + $GLOBALS['cache_request']['post'][$element][$subElement] = $value; } // END - if // Return value @@ -218,7 +218,7 @@ function setPostRequestParameter ($element, $value) { } // Update cache - $GLOBALS['cache_request']['request_post'][$element][null] = $value; + $GLOBALS['cache_request']['post'][$element][null] = $value; } // Checks wether a form was sent. If so, the $_POST['ok'] element must be set diff --git a/inc/session-functions.php b/inc/session-functions.php index 7bfb18c901..f71f3e0796 100644 --- a/inc/session-functions.php +++ b/inc/session-functions.php @@ -46,7 +46,8 @@ function setSession ($var, $value) { if (isCssOutputMode()) return true; // Trim value and session variable - $var = trim(secureString($var)); $value = trim($value); + $var = trim(secureString($var)); + $value = trim($value); // Is the session variable set? if (('' . $value . '' == '') && (isSessionVariableSet($var))) { diff --git a/inc/stylesheet.php b/inc/stylesheet.php index 7cd2a38cf4..81ee2dca3e 100644 --- a/inc/stylesheet.php +++ b/inc/stylesheet.php @@ -109,8 +109,11 @@ if ((isCssOutputMode()) || (getConfig('css_php') == 'DIRECT')) { if ((isInstallationPhase())) { // Default theme first $newTheme = 'default'; - if (isGetRequestParameterSet('theme')) $newTheme = getRequestParameter('theme'); - if (isPostRequestParameterSet('theme')) $newTheme = secureString(postRequestParameter('theme')); + if (isPostRequestParameterSet('theme')) { + $newTheme = postRequestParameter('theme'); + } elseif (isGetRequestParameterSet('theme')) { + $newTheme = getRequestParameter('theme'); + } $OUT .= '?theme=' . $newTheme . '&installing=1'; } else { // Add SVN revision to bypass caching problems diff --git a/templates/de/emails/admin/admin_transfer_points.tpl b/templates/de/emails/admin/admin_transfer_points.tpl index 0529f02805..eba85cf5af 100644 --- a/templates/de/emails/admin/admin_transfer_points.tpl +++ b/templates/de/emails/admin/admin_transfer_points.tpl @@ -24,7 +24,7 @@ Verwendungszweck: $content[reason] Transaktionsnummer: $content[trans_id] ------------------------------ -Diese beiden Mitglieder können sich die Überweisung noch $content[expires] in ihrem Mitgliedsbereich ansehen. Danach wird der Eintrag bei installierter autopurge-Erweiterung automatisch entfernt. +Diese beiden Mitglieder können sich die Überweisung noch {%config,createFancyTime=transfer_age%} in ihrem Mitgliedsbereich ansehen. Danach wird der Eintrag bei installierter autopurge-Erweiterung automatisch entfernt. Mit freundlichen Grüßen, Ihr {?MAIN_TITLE?} Script diff --git a/templates/de/emails/member/member_beg.tpl b/templates/de/emails/member/member_beg.tpl index a66b7c95ee..5c8442d5b0 100644 --- a/templates/de/emails/member/member_beg.tpl +++ b/templates/de/emails/member/member_beg.tpl @@ -1,6 +1,6 @@ Hallo {%user,gender,translateGender=$userid%} {%user,surname=$userid%} {%user,family=$userid%}, -Bei der monatlichen Bettel-Rallye haben Sie soeben Ihre {%pipe,translateComma=$content[points]%} {?POINTS?} gewonnen! +Bei der monatlichen Bettel-Rallye haben Sie soeben Ihre {%pipe,translateComma=$content[beg_points]%} {?POINTS?} gewonnen! Herzlichen Glückwunsch! diff --git a/templates/de/emails/member/member_transfer_recipient.tpl b/templates/de/emails/member/member_transfer_recipient.tpl index e2cddb8bd6..eaf2f9ad13 100644 --- a/templates/de/emails/member/member_transfer_recipient.tpl +++ b/templates/de/emails/member/member_transfer_recipient.tpl @@ -15,7 +15,7 @@ Verwendungszweck: $content[reason] Transaktionsnummer: $content[trans_id] ------------------------------ -Sie können diese Transaktion $content[expires] noch im Mitgliedsbereich unter "{?POINTS?}-Transfer" nachvollziehen. +Sie können diese Transaktion {%config,createFancyTime=transfer_age%} noch im Mitgliedsbereich unter "{?POINTS?}-Transfer" nachvollziehen. Mit freundlichen Grüßen, Ihr {?MAIN_TITLE?} Team diff --git a/templates/de/emails/member/member_transfer_sender.tpl b/templates/de/emails/member/member_transfer_sender.tpl index a0a8b0ff09..b12569c5e3 100644 --- a/templates/de/emails/member/member_transfer_sender.tpl +++ b/templates/de/emails/member/member_transfer_sender.tpl @@ -15,7 +15,7 @@ Verwendungszweck: $content[reason] Transaktionsnummer: $content[trans_id] ------------------------------ -Sie können diese Transaktion $content[expires] noch im Mitgliedsbereich unter "{?POINTS?}-Transfer" nachvollziehen. +Sie können diese Transaktion {%config,createFancyTime=transfer_age%} noch im Mitgliedsbereich unter "{?POINTS?}-Transfer" nachvollziehen. Mit freundlichen Grüßen, Ihr {?MAIN_TITLE?} Team diff --git a/templates/de/html/member/member_list_beg_row.tpl b/templates/de/html/member/member_list_beg_row.tpl index 6812612af1..6075586f5a 100644 --- a/templates/de/html/member/member_list_beg_row.tpl +++ b/templates/de/html/member/member_list_beg_row.tpl @@ -1,6 +1,6 @@ $content[cnt] $content[userid] - {%pipe,translateComma=$content[points]%} + {%pipe,translateComma=$content[beg_points]%} $content[last_online] -- 2.39.5