Is this basic bbcode safe?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

User avatar
WaldoMonster
Forum Contributor
Posts: 225
Joined: Mon Apr 19, 2004 6:19 pm
Contact:

Post by WaldoMonster »

feyd wrote:Pull the whole
  • in as a string and parse it separately then replace as needed.
Thanks, here is the result:

Code: Select all

<?php
function bbcode($string)
{
$bbcode = array(
	'/\[br\]/s',
	'/\[b\](.*?)\[\/b\]/s',
	'/\[i\](.*?)\[\/i\]/s',
	'/\[url=(.*?)\](.*?)\[\/url\]/s');
$html = array(
	'<br>',
	'<strong>$1</strong>',
	'<em>$1</em>',
	'<a href="$1">$2</a>');

$string = htmlentities($string);
$string = preg_replace($bbcode, $html, $string);
$string = preg_replace_callback('/\[list\](.*?)\[\/list\]/s', 'bblist', $string);
return $string;
}


function bblist($maches)
{
$list = '';
$list_array = explode('[*]', $maches[1]);
foreach ($list_array as $key => $value)
	{
	if ($key == 0)	$list .= $value;
	else			$list .= '<li>' . $value . '</li>';
	}
return '<ul>' . $list . '</ul>';
}


echo bbcode('
[list]
no li here
[*]one
[*]two[br]
new line without li
[*]close[/list][/list]
');
?>
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

It should be noted that your code won't handle nesting of tags (still.)
User avatar
WaldoMonster
Forum Contributor
Posts: 225
Joined: Mon Apr 19, 2004 6:19 pm
Contact:

Post by WaldoMonster »

feyd wrote:It should be noted that your code won't handle nesting of tags (still.)
It handles this example without any problem:

Code: Select all

echo bbcode('
[list]
	[*][i]italic [b]bold + italic[/b] italic[/i][br]
	[*]line[br]
	continue
	[*][url=www.test.com/test.php?action=action&option=option][b]bold[/b] url[/url]
	[*][b]bold
	[*][i]bold + italic[/i][/b]
[/list]');
Can you give an example that doesn’t work?
I have posted it in a code block because the php block didn't handle the url section correctly.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

WaldoMonster wrote:Can you give an example that doesn’t work?
Off the top of my head, no, but I know that I ran into problems dealing with the order that the tags were dealt with. You'll have to encounter one before you can really notice it.

Also, if you ever miss a tag (which is easy to do with bbCode.. forgetting the slash on a closing tag and such), the result won't be at all elegant.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Nested lists will "break," as will double bold/italic tags.
User avatar
WaldoMonster
Forum Contributor
Posts: 225
Joined: Mon Apr 19, 2004 6:19 pm
Contact:

Post by WaldoMonster »

feyd wrote:Nested lists will "break," as will double bold/italic tags.
Ok, now I see the limitations.
The double bold or italic and nestled lists can be fixed by repeating the regular expression as many times as there are bold, italic or list pairs.
Or is there a better solution?

I was curious what phpbb would do in those situations.
It handles nestled list forms correctly.
But fails on double bold etc.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

phpBB does bbcode is a MASSIVE process that takes multiple steps. The basic tags, such as bold, are handled similar to yours. For items that are expected to be nested they use a string parser, if memory serves.
User avatar
WaldoMonster
Forum Contributor
Posts: 225
Joined: Mon Apr 19, 2004 6:19 pm
Contact:

Post by WaldoMonster »

feyd wrote:It should be noted that your code won't handle nesting of tags (still.)
For my situation I can live with those limitations.

I have earlier tried to understand regular expression and found it very difficult to understand.
Somehow I now get the hang of it.
What helped me very much is the excellent RegexBuddy tool.

I have added a detection that will open url starting with http,https or ftp in a new windows and open relative php scripts in the same window.
I also have added email address support.
Here is what I came up so far:

Code: Select all

function bbcode($string)
{
$bbcode = array(
	'#\[br\]#s',
	'#\[b\](.*?)\[\/b\]#s',
	'#\[i\](.*?)\[\/i\]#s',
	'#\[url=([a-z]+\.php(?:\?.*)?)\](.*?)\[\/url\]#s',
	'#\[url\]((?:http|https|ftp)://.*?)\[\/url\]#s',
	'#\[url=((?:http|https|ftp)://.*?)\](.*?)\[\/url\]#s',
	'#\[email\]([a-z0-9._%-]+@[a-z0-9.-]+\.[a-z]{2,4})\[\/email\]#si');
$replace = array(
	'<br>',
	'<strong>$1</strong>',
	'<em>$1</em>',
	'<a href="$1">$2</a>',
	'<a href="$1" target="_blank">$1</a>',
	'<a href="$1" target="_blank">$2</a>',
	'<a href="mailto:$1">$1</a>');

$string = htmlentities($string);
$string = preg_replace($bbcode, $replace, $string);
$string = preg_replace_callback('#\[list\](.*?)\[\/list\]#s', 'bblist', $string);
return $string;
}

function bblist($maches)
{
$list = '';
$list_array = explode('[*]', $maches[1]);
foreach ($list_array as $key => $value)
	{
	if ($key == 0)	$list .= $value;
	else			$list .= '<li>' . $value . '</li>';
	}
return '<ul>' . $list . '</ul>';
}
Post Reply