MyBB Community Forums

Full Version: Injection or bad in anyway?
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
I need to programmatically create a thread with some data in it and currently I achieve it in the following way:

            // Set up posthandler.
            require_once MYBB_ROOT."inc/datahandlers/post.php";
            $posthandler = new PostDataHandler("insert");
            $posthandler->action = "thread";

            $usename = $db->escape_string($product['name']);
            $safename = Slug($product['name']);
            $tmessage = htmlspecialchars_uni("[color=#ff9933][size=x-large]some text {$usename}![/size][/color]\n\nmore text\n\n[url={$mybb->settings['bburl']}/{$product['id']}-{$safename}/][color=lightyellow]more text![/color][/url]");

            if ($product['type'] == 'BETA')
            {
                $mybb->input['prefix'] = 13;
            }
            elseif($product['type'] == 'PAID')
            {
                $mybb->input['prefix'] = 11;
            }
            elseif($product['type'] == 'FREE')
            {
                $mybb->input['prefix'] = 10;
            }
            else
            {
                $mybb->input['prefix'] = 0;
            }

            // Set the thread data that came from the input to the $thread array.
            $new_thread = array(
                "fid" => $product['pid'],
                "subject" => $db->escape_string($product['name']."'s support!"),
                "prefix" => $mybb->input['prefix'],
                "icon" => 3,
                "uid" => $product['uid'],
                "username" => $db->escape_string($product['username']),
                "message" => $tmessage,
                "lastposter" => $db->escape_string($product['username']),
                "views" => 0,
                "replies" => 0,
                "visible" => 0,
                "notes" => ''
            );

            $new_thread['options'] = array(
                "signature" => 1,
                "subscriptionmethod" => 1,
                "disablesmilies" => 0
            );
            $posthandler->set_data($new_thread);
            $valid_thread = $posthandler->validate_thread();

            $post_errors = array();
            $errors = array();
            // Fetch friendly error messages if this is an invalid thread
            if(!$valid_thread)
            {
                $post_errors = $posthandler->get_friendly_errors();
                if(count($post_errors) > 0)
                {
                    $errors[] = inline_error($post_errors);
                }
            }

            if (empty($errors))
            {
                $thread_info = $posthandler->insert_thread();
                $tid = $thread_info['tid'];
            }

It does work, and I only have 1 concern, I am not using "escape_string" on the message field, the message field is mainly predefined by me with the exception of 2 entries the URL and the username.

I do use:
            $usename = $db->escape_string($product['name']);
            $safename = Slug($product['name']);

Just in case this is my Slug function:
function Slug($string)
{
    return strtolower(trim(preg_replace('~[^0-9a-z]+~i', '-', html_entity_decode(preg_replace('~&([a-z]{1,2})(?:acute|cedil|circ|grave|lig|orn|ring|slash|th|tilde|uml);~i', '$1', htmlentities($string, ENT_QUOTES, 'UTF-8')), ENT_QUOTES, 'UTF-8')), '-'));
}
To make both the username and product name safe and I was wondering if interpolating it inside my $tmessage could cause a security issue.

Thanks.
Escape it.
Escape everything which goes into the database. No exceptions.
(2015-10-23, 11:21 PM)Omar G. Wrote: [ -> ]Escape it.

When I escape $tmessage I can never get it to display the newlines, the thread doesn't read it or parse it after posted. How can I make the new lines work?

Like it creates the thread fine but newlines show up as \n\n since the escape will escape em.
Use nl2br to make new lines actually show a new line.
(2015-10-24, 01:04 PM)dragonexpert Wrote: [ -> ]Use nl2br to make new lines actually show a new line.

Well I don't want to change the code of how a thread is read, I want to figure out how to properly insert the data without having to manipulate where it is read.

Otherwise I would have a major headache figuring out why the whole forum is not reading threads in a different way it was.

When I manually changed my $tmessage from \n to <br> it simple "parsed" it as it is considered as HTML and result was &qt;br&lt;, which is technically what the nl2br would do, turn it in <br>.

I further checked the datahandler and function_post and I don't see nl2br being used at all.
Did you tried..
/*~~*/
$tmessage = "[color=#ff9933][size=x-large]some text {$usename}![/size][/color]

more text

[url={$mybb->settings['bburl']}/{$product['id']}-{$safename}/][color=lightyellow]more text![/color][/url]";
/*~~*/
yes, it becomes escaped \n as well, I am trying to find what my insertion code is missing that properly makes new lines accepted.

Ok I think I understood it? This is from the newthread.php:

	// Set up posthandler.
	require_once MYBB_ROOT."inc/datahandlers/post.php";
	$posthandler = new PostDataHandler("insert");
	$posthandler->action = "thread";

	// Set the thread data that came from the input to the $thread array.
	$new_thread = array(
		"fid" => $forum['fid'],
		"subject" => $mybb->get_input('subject'),
		"prefix" => $mybb->get_input('threadprefix', MyBB::INPUT_INT),
		"icon" => $mybb->get_input('icon', MyBB::INPUT_INT),
		"uid" => $uid,
		"username" => $username,
		"message" => $mybb->get_input('message'),
		"ipaddress" => $session->packedip,
		"posthash" => $mybb->get_input('posthash')
	);
$posthandler->set_data($new_thread);

escapes are not used at all in there, then further looking at the post handler the escaping is automatically done there.

			$this->post_insert_data = array(
				"subject" => $db->escape_string($thread['subject']),
				"icon" => (int)$thread['icon'],
				"username" => $db->escape_string($thread['username']),
				"dateline" => (int)$thread['dateline'],
				"message" => $db->escape_string($thread['message']),
				"ipaddress" => $db->escape_binary(my_inet_pton(get_ip())),
				"includesig" => $thread['options']['signature'],
				"smilieoff" => $thread['options']['disablesmilies'],
				"visible" => $visible
			);

So I am technically double escaping it hence why it doesn't work. So in the outside layer I don't have to worry about escaping it and when I send it to the post handler it will do all the work?

Did I get it right?
Yes I think you are correct. The insert_post method already escapes everything.