View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0016989 | phpList 3 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 | ||
| 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. | ||||
|
|
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. |
|
|
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? |
|
|
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. |
|
|
yes, very true. Sorry, I was the one confused. |
