From f6d6d1b185fffa84eb6655b209e5c3249b0e1643 Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Tue, 31 Aug 2021 00:11:14 +0200 Subject: [PATCH] Deprecate `e_parse::toJS()` `e_parse::toJS()`, documented with the description > Convert text blocks which are to be embedded within JS , does not protect strings from injections, which appears to be its primary use. Additionally, it performs multiple unrelated string modifications: * Replace Windows line breaks with a literal `\\n` (which would later be parsed as `\n` in JavaScript/JSON) * Does not modify Unix line breaks (`\n`), which is inconsistent with the Windows line break behavior * Removes HTML tags * Replaces HTML entities as `htmlentities()` does This method cannot be fixed because its usages are inconsistent. Most notably, some usages surround the method's output in single quotes while others surround it with double quotes. Strings cannot be JSON-encoded without confounding quotation mark styles. All core usages of `e_parse::toJS()` have been replaced with alternatives, which are also documented in the method's DocBlock. Fixes: #4546 --- contact.php | 8 ++-- e107_admin/administrator.php | 2 +- e107_admin/lancheck.php | 2 +- e107_admin/language.php | 2 +- e107_handlers/e_parse_class.php | 7 ++++ e107_handlers/form_handler.php | 3 +- e107_handlers/menumanager_class.php | 2 +- e107_handlers/message_handler.php | 4 +- e107_plugins/download/download_shortcodes.php | 16 ++++---- e107_plugins/download/includes/admin.php | 39 +++++++++++++++---- e107_plugins/forum/forum_viewforum.php | 2 +- e107_plugins/forum/forum_viewtopic.php | 4 +- e107_plugins/gsitemap/admin_config.php | 2 +- .../login_menu/login_menu_shortcodes.php | 2 +- e107_plugins/newsletter/admin_config.php | 6 +-- .../newsletter/newsletter_legacy_menu.php | 4 +- e107_plugins/poll/admin_config.php | 2 +- 17 files changed, 68 insertions(+), 39 deletions(-) diff --git a/contact.php b/contact.php index c6ce35c09..582515f90 100644 --- a/contact.php +++ b/contact.php @@ -134,24 +134,24 @@ class contact_front // Check Image-Code if(isset($_POST['rand_num']) && ($sec_img->invalidCode($_POST['rand_num'], $_POST['code_verify']))) { - $error .= LAN_CONTACT_15 . "\\n"; + $error .= LAN_CONTACT_15 . "\n"; } // Check message body. if(strlen(trim($body)) < 15) { - $error .= LAN_CONTACT_12 . "\\n"; + $error .= LAN_CONTACT_12 . "\n"; } // Check subject line. if(isset($_POST['subject']) && strlen(trim($subject)) < 2) { - $error .= LAN_CONTACT_13 . "\\n"; + $error .= LAN_CONTACT_13 . "\n"; } if(!strpos(trim($sender), "@")) { - $error .= LAN_CONTACT_11 . "\\n"; + $error .= LAN_CONTACT_11 . "\n"; } // No errors - so proceed to email the admin and the user (if selected). diff --git a/e107_admin/administrator.php b/e107_admin/administrator.php index d7432e9ec..5dfdbabb3 100644 --- a/e107_admin/administrator.php +++ b/e107_admin/administrator.php @@ -160,7 +160,7 @@ function show_admins() { $text .= " ".$frm->submit_image("edit_admin[{$row['user_id']}]", 'edit', 'edit', LAN_EDIT)." - ".$frm->submit_image("del_admin[{$row['user_id']}]", 'del', 'delete', $tp->toJS(ADMSLAN_59."? [".$row['user_name']."]"))." + ".$frm->submit_image("del_admin[{$row['user_id']}]", 'del', 'delete', ADMSLAN_59."? [".$row['user_name']."]")." "; } diff --git a/e107_admin/lancheck.php b/e107_admin/lancheck.php index d32d77944..4c15bae78 100644 --- a/e107_admin/lancheck.php +++ b/e107_admin/lancheck.php @@ -1027,7 +1027,7 @@ class lancheck $just_go_diz = (deftrue('LAN_CHECK_20')) ? LAN_CHECK_20 : "Generate Language Pack"; $lang_sel_diz = (deftrue('LAN_CHECK_21')) ? LAN_CHECK_21 : "Verify Again"; - $lan_pleasewait = (deftrue('LAN_PLEASEWAIT')) ? $tp->toJS(LAN_PLEASEWAIT) : "Please Wait"; + $lan_pleasewait = (deftrue('LAN_PLEASEWAIT')) ? LAN_PLEASEWAIT : "Please Wait"; $message .= "

diff --git a/e107_admin/language.php b/e107_admin/language.php index 24683daa6..1ab241702 100644 --- a/e107_admin/language.php +++ b/e107_admin/language.php @@ -271,7 +271,7 @@ if(!empty($_GET['iframe'])) $release_diz = defset("LANG_LAN_30","Release Date"); $compat_diz = defset("LANG_LAN_31", "Compatibility"); - $lan_pleasewait = (deftrue('LAN_PLEASEWAIT')) ? $tp->toJS(LAN_PLEASEWAIT) : "Please Wait"; + $lan_pleasewait = (deftrue('LAN_PLEASEWAIT')) ? LAN_PLEASEWAIT : "Please Wait"; $text = "
diff --git a/e107_handlers/e_parse_class.php b/e107_handlers/e_parse_class.php index 4ce21ae04..bc9806a87 100644 --- a/e107_handlers/e_parse_class.php +++ b/e107_handlers/e_parse_class.php @@ -1871,9 +1871,16 @@ class e_parse * * @param string|array $stringarray * @return string + * @deprecated v2.3.1 This method will not escape a string properly for use as a JavaScript or JSON string. Use + * {@see e_parse::toJSON()} instead. When using {@see e_parse::toJSON()}, do not surround its output + * with quotation marks, and do not attempt to escape sequences like "\n" as "\\n". If HTML tags need to + * be removed, consider {@see e_parse::toText()} separately. If the text needs to be used in an HTML + * tag attribute (e.g. <a onclick="ATTRIBUTE"></a>), surround the string with + * {@see e_parse::toAttribute()} and either single-quote or double-quote the attribute value. */ public function toJS($stringarray) { + trigger_error('' . __METHOD__ . ' is deprecated. See method DocBlock for alternatives.', E_USER_WARNING); // NO LAN $search = array("\r\n", "\r", '
', "'"); $replace = array("\\n", '', "\\n", "\'"); diff --git a/e107_handlers/form_handler.php b/e107_handlers/form_handler.php index 7413e0743..fa5bc815d 100644 --- a/e107_handlers/form_handler.php +++ b/e107_handlers/form_handler.php @@ -3910,8 +3910,7 @@ var_dump($select_options);*/ // foreach ($options as $option => $optval) { - $optval = trim((string) $optval); - $optval = htmlspecialchars($optval, ENT_COMPAT | ENT_HTML401, 'UTF-8', false); + $optval = htmlspecialchars(trim((string) $optval), ENT_COMPAT | ENT_HTML401, 'UTF-8', false); switch ($option) { diff --git a/e107_handlers/menumanager_class.php b/e107_handlers/menumanager_class.php index 7bb1354c2..acfba3801 100644 --- a/e107_handlers/menumanager_class.php +++ b/e107_handlers/menumanager_class.php @@ -1329,7 +1329,7 @@ class e_menuManager { if(isset($pref['sitetheme_layouts'][$layout]['menuPresets'])) { - $text .= "toJS(MENLAN_41)."')\" />

\n"; // Use Menu Presets + $text .= "toAttribute($tp->toJSON(MENLAN_41)).")\" />

\n"; // Use Menu Presets $text .= ""; } $text .= ""; diff --git a/e107_handlers/message_handler.php b/e107_handlers/message_handler.php index cc1f7d1b3..f395c32d4 100644 --- a/e107_handlers/message_handler.php +++ b/e107_handlers/message_handler.php @@ -1152,11 +1152,11 @@ $SYSTEM_DIRECTORY = "e107_system/"; case "ALERT": $message = isset($emessage[$message]) ? $emessage[$message] : $message; - echo "\n"; exit; + echo "\n"; exit; break; case "P_ALERT": - echo "\n"; + echo "\n"; break; case 'POPUP': diff --git a/e107_plugins/download/download_shortcodes.php b/e107_plugins/download/download_shortcodes.php index 0818d5fa2..f5806e0ab 100644 --- a/e107_plugins/download/download_shortcodes.php +++ b/e107_plugins/download/download_shortcodes.php @@ -373,15 +373,15 @@ class download_shortcodes extends e_shortcode if ($parm == "request") { - $agreetext = $tp->toJS($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION')); + $agreetext = $tp->toAttribute($tp->toJSON($tp->filter($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'), 'str'))); if ($this->var['download_mirror_type']) { - $text = ($this->pref['agree_flag'] ? "" : ""); + $text = ($this->pref['agree_flag'] ? "" : ""); } else { - $text = ($this->pref['agree_flag'] ? "" : ""); + $text = ($this->pref['agree_flag'] ? "" : ""); } $text .= $tp->toHTML($this->var['download_name'], FALSE, 'TITLE').""; @@ -473,7 +473,7 @@ class download_shortcodes extends e_shortcode $tp = e107::getParser(); $img = ''; - $agreetext = !empty($this->pref['agree_text']) ? $tp->toJS($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION')) : ''; + $agreetext = !empty($this->pref['agree_text']) ? $tp->toAttribute($tp->toJSON($tp->filter($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'), 'str'))) : ''; if(defined('IMAGE_DOWNLOAD')) { @@ -493,7 +493,7 @@ class download_shortcodes extends e_shortcode else { $url = $tp->parseTemplate("{DOWNLOAD_REQUEST_URL}",true, $this); // $this->sc_download_request_url(); - return (!empty($this->pref['agree_flag']) ? "{$img}" : "{$img}"); + return (!empty($this->pref['agree_flag']) ? "{$img}" : "{$img}"); // return ($this->pref['agree_flag'] ? "{$img}" : "{$img}"); } @@ -653,7 +653,7 @@ class download_shortcodes extends e_shortcode if (!empty($this->pref['agree_flag'])) { - return "toJS($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'))."');\" title='".LAN_dl_46."'>".$this->var['download_name'].""; + return "toAttribute($tp->toJSON($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'))).");\" title='".LAN_dl_46."'>".$this->var['download_name'].""; // return "toJS($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'))."');\" title='".LAN_dl_46."'>".$dl['download_name'].""; } else @@ -799,7 +799,7 @@ class download_shortcodes extends e_shortcode if(!empty($this->pref['agree_flag'])) { - $click = " onclick='return confirm(\"".$tp->toJS($tp->toHTML($this->pref['agree_text'],true,'emotes, no_tags'))."\")'"; + $click = " onclick='return confirm(".$tp->toAttribute($tp->toJSON($tp->toHTML($this->pref['agree_text'],true,'emotes, no_tags'))).")'"; } $url = $url = $tp->parseTemplate("{DOWNLOAD_REQUEST_URL}",true,$this); //$this->sc_download_request_url(); @@ -955,7 +955,7 @@ class download_shortcodes extends e_shortcode $tp = e107::getParser(); $img = ''; - $click = !empty($this->pref['agree_text']) ? " onclick='return confirm(\"".$tp->toJS($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'))."\")'" : ''; + $click = !empty($this->pref['agree_text']) ? " onclick='return confirm(".$tp->toAttribute($tp->toJSON($tp->toHTML($this->pref['agree_text'],FALSE,'DESCRIPTION'))).")'" : ''; if(defined('IMAGE_DOWNLOAD')) { diff --git a/e107_plugins/download/includes/admin.php b/e107_plugins/download/includes/admin.php index 0501be4c1..2fd90a379 100644 --- a/e107_plugins/download/includes/admin.php +++ b/e107_plugins/download/includes/admin.php @@ -410,6 +410,20 @@ $columnInfo = array( 'pref_name' => array('title'=> 'name', 'type' => 'text', 'data' => 'string', 'validate' => 'regex', 'rule' => '#^[\w]+$#i', 'help' => 'allowed characters are a-zA-Z and underscore') ); + /** + * @var e_parse + */ + private $tp; + + /** + * @inheritDoc + */ + public function __construct($request, $response, $params = array()) + { + parent::__construct($request, $response, $params); + $this->tp = e107::getParser(); + } + public function observe() { @@ -796,7 +810,7 @@ $columnInfo = array( $text .= ''.$tp->toHTML($row['download_category_name']).''; $text .= ' '.ADMIN_EDIT_ICON.' - toJS(DOWLAN_33.' [ID: '.$row["download_id"].' ]').'") \'/> + getJsConfirm($row["download_id"]) .') \'/> '; $text .= ''; } @@ -888,7 +902,7 @@ $columnInfo = array( $text .= ''.$tp->toHTML($row['download_url']).''; $text .= ' '.ADMIN_EDIT_ICON.' - toJS(DOWLAN_33.' [ID: '.$row["download_id"].' ]').'") \'/> + getJsConfirm($row["download_id"]) .') \'/> '; $text .= ''; } @@ -944,7 +958,7 @@ $columnInfo = array( } $text .= ' '.ADMIN_EDIT_ICON.' - toJS(DOWLAN_33.' [ID: '.$row["download_id"].' ]').'") \'/> + getJsConfirm($row["download_id"]) .') \'/> '; $text .= ''; } @@ -996,7 +1010,7 @@ $columnInfo = array( } $text .= ' '.ADMIN_EDIT_ICON.' - toJS(DOWLAN_33.' [ID: '.$row["download_id"].' ]').'") \'/> + getJsConfirm($row["download_id"]) .') \'/> '; $text .= ''; } @@ -1047,7 +1061,7 @@ $columnInfo = array( $text .= ''; $text .= ' '.ADMIN_EDIT_ICON.' - toJS(DOWLAN_33.' [ID: '.$row["download_id"].' ]').'") \'/> + getJsConfirm($row["download_id"]) .') \'/> '; $text .= ''; } @@ -2444,9 +2458,18 @@ $columnInfo = array( } return $ret; } - - - + + /** + * @param string|int $download_id + * @return string + */ + private function getJsConfirm($download_id) + { + $tp = $this->tp; + return $tp->toAttribute($tp->toJSON(DOWLAN_33 . ' [ID: ' . $download_id . ' ]')); + } + + } class download_main_admin_form_ui extends e_admin_form_ui diff --git a/e107_plugins/forum/forum_viewforum.php b/e107_plugins/forum/forum_viewforum.php index 5489859d8..b6995042b 100644 --- a/e107_plugins/forum/forum_viewforum.php +++ b/e107_plugins/forum/forum_viewforum.php @@ -450,7 +450,7 @@ function init() echo ""; diff --git a/e107_plugins/forum/forum_viewtopic.php b/e107_plugins/forum/forum_viewtopic.php index ba0bbc2f6..e2994d468 100644 --- a/e107_plugins/forum/forum_viewtopic.php +++ b/e107_plugins/forum/forum_viewtopic.php @@ -396,9 +396,9 @@ else echo ""; diff --git a/e107_plugins/gsitemap/admin_config.php b/e107_plugins/gsitemap/admin_config.php index 1f499e404..f5e7f208f 100644 --- a/e107_plugins/gsitemap/admin_config.php +++ b/e107_plugins/gsitemap/admin_config.php @@ -813,7 +813,7 @@ class gsitemap
- +
diff --git a/e107_plugins/login_menu/login_menu_shortcodes.php b/e107_plugins/login_menu/login_menu_shortcodes.php index 11a0e1bd7..a2d04a135 100755 --- a/e107_plugins/login_menu/login_menu_shortcodes.php +++ b/e107_plugins/login_menu/login_menu_shortcodes.php @@ -529,7 +529,7 @@ e107::getLanguage()->bcDefs($bcDefs); $srch = array("
","'"); $rep = array("\\n","\'"); return ""; } else diff --git a/e107_plugins/newsletter/admin_config.php b/e107_plugins/newsletter/admin_config.php index adde54236..4d3892ba9 100644 --- a/e107_plugins/newsletter/admin_config.php +++ b/e107_plugins/newsletter/admin_config.php @@ -157,7 +157,7 @@ class newsletter ".((substr_count($data['newsletter_subscribers'], chr(1))!= 0)?"".substr_count($data['newsletter_subscribers'], chr(1))."":substr_count($data['newsletter_subscribers'], chr(1)))." ".ADMIN_EDIT_ICON." - toJS(LAN_CONFIRMDEL." [ID: ".$data['newsletter_id']." ]")."') \"/> + toAttribute($tp->toJSON(LAN_CONFIRMDEL." [ID: ".$data['newsletter_id']." ]")).") \"/> @@ -206,8 +206,8 @@ class newsletter ".$data['newsletter_id']." ".$data['newsletter_issue']." [ ".$data['newsletter_parent']." ] ".$data['newsletter_title']." - ".($data['newsletter_flag'] ? LAN_YES : "toJS(NLLAN_18)."') \" />")." - ".ADMIN_EDIT_ICON."toJS(NLLAN_19." [ID: ".$data['newsletter_id']." ]")."') \"/> + ".($data['newsletter_flag'] ? LAN_YES : "toAttribute($tp->toJSON(NLLAN_18)).") \" />")." + ".ADMIN_EDIT_ICON."toAttribute($tp->toJSON(NLLAN_19." [ID: ".$data['newsletter_id']." ]")).") \"/> diff --git a/e107_plugins/newsletter/newsletter_legacy_menu.php b/e107_plugins/newsletter/newsletter_legacy_menu.php index 202ebcbf8..a65e9486f 100644 --- a/e107_plugins/newsletter/newsletter_legacy_menu.php +++ b/e107_plugins/newsletter/newsletter_legacy_menu.php @@ -84,13 +84,13 @@ foreach($newsletterArray as $nl) if(preg_match("#".chr(1).USERID."(".chr(1)."|$)#si", $nl['newsletter_subscribers'])) { $text .= NLLAN_48."

- toJS(NLLAN_49)."') \" /> + toAttribute($tp->toJSON(NLLAN_49)).") \" /> "; } else { $text .= NLLAN_50." ".USEREMAIL.")

- toJS(NLLAN_53)."') \" /> + toAttribute($tp->toJSON(NLLAN_53))."') \" /> "; } $nl_count = $sql->count('newsletter', "(*)", "WHERE newsletter_parent='".$nl['newsletter_id']."' AND newsletter_flag='1'"); diff --git a/e107_plugins/poll/admin_config.php b/e107_plugins/poll/admin_config.php index e24d7ae99..476a45468 100644 --- a/e107_plugins/poll/admin_config.php +++ b/e107_plugins/poll/admin_config.php @@ -211,7 +211,7 @@ function poll_list() $text .= " - + "; }