View Issue Details

Jump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0016989phpList 3 applicationInterface - Backendpublic17-12-13 12:1904-01-14 23:35
Reporterduncanc Assigned To 
PrioritynormalSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Product Version3.0.5 
Fixed in Version3.0.6 
Summary0016989: Some confusion with added slashes in message_data table
DescriptionI am developing a plugin that adds a tab to the message page and uses the [] notation on field names so that php automatically builds an array
e.g name="MCAPlugin[zzz][0]" for one field, name="MCAPlugin[zzz][1]" for a second means $_POST has an entry MCAPlugin that is an array.

This has highlighted a magic quotes problem when a field value contains a single quote.
The function Sql_Replace() used by setMessageData() escapes its parameter therefore, because all $_GET, $_POST values have already had slashes added in magic_quotes.php, the \ character remains in the field value when added to the message_data table.

The problem is in the code that retrieves the message data, function loadMessageData in lib.php has these lines:

    if (strpos($row['data'],'SER:') === 0) {
      $data = substr($row['data'],4);
      $data = @unserialize(stripslashes($data));
    } else {

The use of stripslashes changes the data that was serialised and makes unserialize() fail.
unserialize() works when the stripslashes() call is removed, but then the resultant array has unnecessary \ characters that should be removed.

Rather than remove them after unserialising, I suggest removing the slashes before serialising in file actions/storemessage.php.
That needs a recursive function to remove slashes, the opposite of the function addSlashesArray() in magic_quotes.php.

So this section in storemessage.php should remove slashes recursively. I'm also not sure that the get_magic_quotes_gpc() is needed because all $_REQUEST elements have already had slashes added if magic quotes is off.

## remember all data entered
foreach ($_REQUEST as $key => $val) {
/*
  print $key .' '.$val;
*/
  setMessageData($id,$key,$val);
  if (get_magic_quotes_gpc()) {
    if (is_string($val)) {
      $messagedata[$key] = stripslashes($val);
    } else {
      $messagedata[$key] = $val;
    }
  } else {
    $messagedata[$key] = $val;
  }
}

TagsNo tags attached.

Activities

duncanc

01-01-14 13:03

updater   ~0052504

See GitHub pull request 19.
I have made the simplest change of removing slashes after deserialising as trying to remove slashes before serialising seems to require more changes.

michiel

03-01-14 00:04

administrator   ~0052505

I've actioned the pull request, but shouldn't it take into account the php.ini setting for magic_quotes or other similar settings? IE if magic_quotes run strip_slashes otherwise not?

duncanc

03-01-14 08:05

updater   ~0052506

Not sure I understand the comment as the previous code was unconditionally removing slashes before deserialising.

But if you are referring to storemessage.php, are you saying that a change also needs to be applied there? The code there is a bit confusing as the messagedata field is populated in the loop at line 46, but then is overwritten at line 63.

Just to clarify my understanding, commonlib/lib/magic_quotes.php, which is included in index.php, already adds slashes to $_POST etc when the magic quotes ini settings are off. So, in theory, all data now has slashes regardless of the magic quotes setting and any further tests are unnecessary.

michiel

04-01-14 23:35

administrator   ~0052507

yes, very true. Sorry, I was the one confused.