From 81140ec8877236050b822517f735b49503bcd44d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 3 Jun 2013 12:15:23 +0200 Subject: [PATCH 1/6] [ticket/8319] Prepend Board URL to LOCAL_URL links to prevent abuse The description says: "The URL must be relative to the topic page and cannot contain a server name or protocol." We now enforce this and will add a new token with the current behaviour back. PHPBB3-8319 --- phpBB/includes/acp/acp_bbcodes.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_bbcodes.php b/phpBB/includes/acp/acp_bbcodes.php index 0644b38eb1..2cb1390c8e 100644 --- a/phpBB/includes/acp/acp_bbcodes.php +++ b/phpBB/includes/acp/acp_bbcodes.php @@ -427,7 +427,15 @@ class acp_bbcodes $fp_replace = str_replace($token, $replace, $fp_replace); $sp_match = str_replace(preg_quote($token, '!'), $sp_tokens[$token_type], $sp_match); - $sp_replace = str_replace($token, '${' . ($n + 1) . '}', $sp_replace); + if ($token_type === 'LOCAL_URL') + { + // Prepend the board url to local relative links + $sp_replace = str_replace($token, generate_board_url() . '/' . '${' . ($n + 1) . '}', $sp_replace); + } + else + { + $sp_replace = str_replace($token, '${' . ($n + 1) . '}', $sp_replace); + } } $fp_match = '!' . $fp_match . '!' . $modifiers; From c0e0c13cf17bac9256e3acb1ea2d67134d6122dd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 3 Jun 2013 11:57:40 +0200 Subject: [PATCH 2/6] [ticket/8319] Add new token RELATIVE_URL to allow foreign relative URL parts PHPBB3-8319 --- phpBB/includes/acp/acp_bbcodes.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/acp/acp_bbcodes.php b/phpBB/includes/acp/acp_bbcodes.php index 2cb1390c8e..02ec8af5b9 100644 --- a/phpBB/includes/acp/acp_bbcodes.php +++ b/phpBB/includes/acp/acp_bbcodes.php @@ -345,6 +345,9 @@ class acp_bbcodes 'LOCAL_URL' => array( '!(' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('relative_url')) . ')!e' => "\$this->bbcode_specialchars('$1')" ), + 'RELATIVE_URL' => array( + '!(' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('relative_url')) . ')!e' => "\$this->bbcode_specialchars('$1')" + ), 'EMAIL' => array( '!(' . get_preg_expression('email') . ')!ie' => "\$this->bbcode_specialchars('$1')" ), @@ -371,6 +374,7 @@ class acp_bbcodes $sp_tokens = array( 'URL' => '(?i)((?:' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('url')) . ')|(?:' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('www_url')) . '))(?-i)', 'LOCAL_URL' => '(?i)(' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('relative_url')) . ')(?-i)', + 'RELATIVE_URL' => '(?i)(' . str_replace(array('!', '\#'), array('\!', '#'), get_preg_expression('relative_url')) . ')(?-i)', 'EMAIL' => '(' . get_preg_expression('email') . ')', 'TEXT' => '(.*?)', 'SIMPLETEXT' => '([a-zA-Z0-9-+.,_ ]+)', From 24bfaa13270bbdeca1e00c95b025b3fd42742633 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 3 Jun 2013 12:37:57 +0200 Subject: [PATCH 3/6] [ticket/8319] Update BBCodes that currently use the LOCAL_URL tag on update To fix http://tracker.phpbb.com/browse/PHPBB3-8319 we changed the second_pass_replace value, so that needs updating for existing ones. PHPBB3-8319 --- phpBB/install/database_update.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 8aa62af7e1..a0ca05a129 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -2204,6 +2204,36 @@ function change_database_data(&$no_updates, $version) _sql($sql, $errored, $error_ary); } + /** + * Update BBCodes that currently use the LOCAL_URL tag + * + * To fix http://tracker.phpbb.com/browse/PHPBB3-8319 we changed + * the second_pass_replace value, so that needs updating for existing ones + */ + $sql = 'SELECT * + FROM ' . BBCODES_TABLE . ' + WHERE bbcode_match ' . $db->sql_like_expression($db->any_char . 'LOCAL_URL' . $db->any_char); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + if (!class_exists('acp_bbcodes')) + { + phpbb_require_updated('includes/acp/acp_bbcodes.' . $phpEx); + } + $bbcode_match = $row['bbcode_match']; + $bbcode_tpl = $row['bbcode_tpl']; + + $acp_bbcodes = new acp_bbcodes(); + $sql_ary = $acp_bbcodes->build_regexp($bbcode_match, $bbcode_tpl); + + $sql = 'UPDATE ' . BBCODES_TABLE . ' + SET ' . $db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE bbcode_id = ' . (int) $row['bbcode_id']; + $db->sql_query($sql); + } + $db->sql_freeresult($result); + $no_updates = false; break; } From 6206d4aa4ea08df513154d8b87d35785f69a9f2a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 3 Jun 2013 12:35:57 +0200 Subject: [PATCH 4/6] [ticket/8319] Add explanation for RELATIVE_URL and update LOCAL_URL The explanation now states that links are prefixed with the board URL. PHPBB3-8319 --- phpBB/includes/acp/acp_bbcodes.php | 4 ++-- phpBB/language/en/acp/posting.php | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/acp/acp_bbcodes.php b/phpBB/includes/acp/acp_bbcodes.php index 02ec8af5b9..ead716b300 100644 --- a/phpBB/includes/acp/acp_bbcodes.php +++ b/phpBB/includes/acp/acp_bbcodes.php @@ -113,8 +113,8 @@ class acp_bbcodes { $template->assign_block_vars('token', array( 'TOKEN' => '{' . $token . '}', - 'EXPLAIN' => $token_explain) - ); + 'EXPLAIN' => ($token === 'LOCAL_URL') ? sprintf($token_explain, generate_board_url() . '/') : $token_explain, + )); } return; diff --git a/phpBB/language/en/acp/posting.php b/phpBB/language/en/acp/posting.php index 9719287c2a..9232be661b 100644 --- a/phpBB/language/en/acp/posting.php +++ b/phpBB/language/en/acp/posting.php @@ -83,7 +83,8 @@ $lang = array_merge($lang, array( 'NUMBER' => 'Any series of digits', 'EMAIL' => 'A valid e-mail address', 'URL' => 'A valid URL using any protocol (http, ftp, etc… cannot be used for javascript exploits). If none is given, “http://” is prefixed to the string.', - 'LOCAL_URL' => 'A local URL. The URL must be relative to the topic page and cannot contain a server name or protocol.', + 'LOCAL_URL' => 'A local URL. The URL must be relative to the topic page and cannot contain a server name or protocol, as links are prefixed with “%s”', + 'RELATIVE_URL' => 'A relative URL. You can use this to match parts of a URL, but be careful: a full URL is a valid relative URL. When you want to use relative URLs of your board, use the LOCAL_URL token.', 'COLOR' => 'A HTML colour, can be either in the numeric form #FF1234 or a CSS colour keyword such as fuchsia or InactiveBorder' ) )); From 9210d735a53e3c4a4de042b49dae361c436268e1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 6 Jul 2013 19:40:25 +0200 Subject: [PATCH 5/6] [ticket/8319] Do not repeat the replacement PHPBB3-8319 --- phpBB/includes/acp/acp_bbcodes.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/phpBB/includes/acp/acp_bbcodes.php b/phpBB/includes/acp/acp_bbcodes.php index ead716b300..31166a56dc 100644 --- a/phpBB/includes/acp/acp_bbcodes.php +++ b/phpBB/includes/acp/acp_bbcodes.php @@ -431,15 +431,11 @@ class acp_bbcodes $fp_replace = str_replace($token, $replace, $fp_replace); $sp_match = str_replace(preg_quote($token, '!'), $sp_tokens[$token_type], $sp_match); - if ($token_type === 'LOCAL_URL') - { - // Prepend the board url to local relative links - $sp_replace = str_replace($token, generate_board_url() . '/' . '${' . ($n + 1) . '}', $sp_replace); - } - else - { - $sp_replace = str_replace($token, '${' . ($n + 1) . '}', $sp_replace); - } + + // Prepend the board url to local relative links + $replace_prepend = ($token_type === 'LOCAL_URL') ? generate_board_url() . '/' : ''; + + $sp_replace = str_replace($token, $replace_prepend . '${' . ($n + 1) . '}', $sp_replace); } $fp_match = '!' . $fp_match . '!' . $modifiers; From 19f7d12328fe1f100cd723fa808a291e634d8737 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 6 Jul 2013 19:56:41 +0200 Subject: [PATCH 6/6] [ticket/8319] Add migration file for update change PHPBB3-8319 --- .../migration/data/30x/local_url_bbcode.php | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 phpBB/includes/db/migration/data/30x/local_url_bbcode.php diff --git a/phpBB/includes/db/migration/data/30x/local_url_bbcode.php b/phpBB/includes/db/migration/data/30x/local_url_bbcode.php new file mode 100644 index 0000000000..f324b8880d --- /dev/null +++ b/phpBB/includes/db/migration/data/30x/local_url_bbcode.php @@ -0,0 +1,57 @@ +db->sql_like_expression($this->db->any_char . 'LOCAL_URL' . $this->db->any_char); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + if (!class_exists('acp_bbcodes')) + { + global $phpEx; + phpbb_require_updated('includes/acp/acp_bbcodes.' . $phpEx); + } + $bbcode_match = $row['bbcode_match']; + $bbcode_tpl = $row['bbcode_tpl']; + + $acp_bbcodes = new acp_bbcodes(); + $sql_ary = $acp_bbcodes->build_regexp($bbcode_match, $bbcode_tpl); + + $sql = 'UPDATE ' . BBCODES_TABLE . ' + SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE bbcode_id = ' . (int) $row['bbcode_id']; + $this->sql_query($sql); + } + $this->db->sql_freeresult($result); + } +}