0016989: Some confusion with added slashes in message_data table - MantisBT

NOTE:: Before reporting an issue, make sure you are running the latest version, currently 3.3.1

View Issue Details [ Jump to Notes ]

[ Print ]

ID

Project

Category

View Status

Date Submitted

Last Update

0016989

phplist application

Interface - Backend

public

17-12-13 12:19

04-01-14 23:35

Reporter

duncanc

Priority

normal

Severity

minor

Reproducibility

sometimes

Status

resolved

Resolution

fixed

Platform

OS

OS Version

Product Version

3.0.5

Target Version

Fixed in Version

3.0.6

Summary

0016989: Some confusion with added slashes in message_data table

Description

I 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;
  }
}

Tags

No tags attached.

Relationships [ Relation Graph ] [ Dependency Graph ]

Notes
(0052504) duncanc (developer) 01-01-14 13:03	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.

(0052505) michiel (manager) 03-01-14 00:04	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?

(0052506) duncanc (developer) 03-01-14 08:05	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.

(0052507) michiel (manager) 04-01-14 23:35	yes, very true. Sorry, I was the one confused.

Copyright © 2000 - 2017 MantisBT Team