MyBB Community Forums

Hello,

In /install/resources/upgrade31.php, these lines :

// Update help documents
	$query = $db->simple_select('helpdocs', 'document', 'hid=\'3\'');
	$helpdoc = $db->fetch_array($query);
	if(my_strpos($helpdoc['document'], ';key={1}') !== false)
	{
		$helpdoc['document'] = str_replace(';key={1}', ';my_post_key={1}', $helpdoc['document']);
	}
	$db->update_query('helpdocs', array('document' => $helpdoc['document']), 'hid=\'3\'');

causes a MySQL error when upgrading a french installation of previous version of MyBB.

Heres's the error :

Quote:MyBB has experienced an internal SQL error and cannot continue.

SQL Error:
1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'êtes pas.<br /> <br /> Les cookies sont des petits documents de texte stocké' at line 2
Query:
UPDATE mybb16_helpdocs SET document='MyBulletinBoard utilise les cookies pour stocker vos informations de connexion, si vous êtes enregistré, et votre dernière visite, si vous ne l'êtes pas.<br /> <br /> Les cookies sont des petits documents de texte stockés sur votre ordinateur ; les cookies de ce forum ne sont utilisés que par ce forum et ne posent aucun problème de sécurité.<br /> <br /> Les cookies de ce forum suivent vos lectures et le moment où vous lisez certains messages.<br /> <br /> Pour supprimer tous les cookies de ce forum, vous pouvez cliquer <a href="misc.php?action=clearcookies&amp;my_post_key={1}">ici</a>.' WHERE hid='3'
Please contact the MyBB Group for technical support.

As you can see, the $helpdoc['document'] with hid=3 contains a single quote wich is not escaped by your code.

Furthermore, hid is a numeric field, so why escape the number?

$query = $db->simple_select('helpdocs', 'document', 'hid=\'3\'');

You should replace with :

$query = $db->simple_select('helpdocs', 'document', 'hid=3');

In french packs, we (on mybb.fr) have decided to replace all single quotes in strings with curly quotes, anywhere, install as well as  language files.
That way, no more headaches with not escaped variables used by Javascript !  

I think that should be also a good idea for all languages, including english.

So I added the following code in upgrade31.php to avoid errors with quotes :

// Replace all occurrence of simple quote with curly quote
    $query = $db->simple_select('helpdocs', 'document,hid');
    while($helpdoc = $db->fetch_array($query)){
      $helpdoc['document'] = str_replace ("'", "’",  $helpdoc['document']);
      $db->update_query('helpdocs', array('document' => $helpdoc['document']), "hid={$helpdoc['hid']}");
    }
    // your code now with 'hid=3' instead of 'hid=\'3\''
    // Update help documents
    $query = $db->simple_select('helpdocs', 'document', 'hid=3');
    // etc.

 

Better fix would be to use $db->escape_string(), but you're right. This is definitely a bug.

Hi,

Thank you for your report. We have pushed this issue to our Github repository for further analysis where you can track our commits and progress with fixing this bug. Discussions regarding this bug may also take place there too.

Follow this link to visit the issue on Github: https://github.com/mybb/mybb/issues/1559

Thanks for contributing to MyBB!

Regards,
The MyBB Group

(2014-10-27, 10:13 PM)spyto Wrote: [ -> ]Furthermore, hid is a numeric field, so why escape the number?

The number is not escaped, just the redundant wrapping apostrophes.. So that part of code is correct, but nonetheless it's a spaghetti - I have completely no idea why some people here keep introducing redundant escape characters like this:

$random = "class=\"test\" selected=\"selected\"";

instead of simplyfying it:

$random = 'class="test" selected="selected"';

Both " and ' are available for wrapping strings in PHP for a good reason. Not to mention heredoc, but that's rather for longer and more complicated lines.

Negatives? It:
a) makes the files a bit bigger
b) makes the lines a bit longer and less readable
c) has a higher possibility of missing something (the escape character) and introducing an error

Positives? None.

So I agree it that 'hid=3' would be much more soothing for eyes.