View Issue Details

IDProjectCategoryView StatusLast Update
0017566phplist applicationMessage Managementpublic07-07-15 14:02
Reporterduncanc 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.10 
Target Versionnext patchFixed in Version3.2.0 
Summary0017566: Message data table contains data unrelated to the message
DescriptionWhen going to the Campaign page to edit a new or existing campaign there was a message displayed:
Illegal double 'xxxxx' value found during parsing

I noticed that the value was actually the tk parameter in the URL. After a bit of digging I found that the message_data table has rows containing not only the tk parameter but also what appear to be cookie variables unrelated to a message and unrelated to phplist (such as wordpress cookie variables).

The particular problem was caused by the tk value looking like a double, e.g. 123e12345 and mysql was trying to parse it as a double.

These rows are a side-effect of trying to capture all the data for the message using $_REQUEST see file admin/actions/storemessage.php.

In some php configurations $_REQUEST includes $_COOKIE, see http://php.net/manual/en/reserved.variables.request.php and the reference to request_order.

To avoid this problem the code should use $_POST + $_GET or array_merge($_GET, $_POST) instead of $_REQUEST. This corresponds to the, now default, setting of 'GP' for request_order.
TagsNo tags attached.

Activities

duncanc

01-01-15 14:21

developer   ~0055795

Last edited: 01-01-15 14:22

View 2 revisions

Looking a bit closer I am not sure why $_GET is used in this processing as it seems to contain only page, id, tk and tab parameters. None of which look to be relevant to the message data. When saving a tab $_POST should contain all the fields on that tab.

This would then remove the problem of the tk value being interpreted as a double.

duncanc

01-01-15 14:28

developer   ~0055796

Function Sql_Query_Params in mysqli.inc appears to be causing the problem with the tk parameter value.

The line
    if (is_numeric($par)) {
is going to treat something like 123e123 as numeric.

Unfortunately this sort of ad-hoc query parameter processing is not going to be very robust. It looks like booleans might not be handled correctly too.

It would be much better to use the proper mysql prepared statements instead.

michiel

03-01-15 12:20

manager   ~0055799

yes, true GET params can probably be ignored.

all params are recorded, so that plugin data is also stored centrally.

michiel

07-07-15 14:02

manager   ~0056343

https://github.com/phpList/phplist3/commit/55009f8755a6f4b28e451f143699259a48c6b6d8