AdminRequestSanitizer Error Log

@wilt, I just found the following warning in my /logs folder; it's issued when the edit_orders script is initially entered (i.e. no $_POST parameters).

PHP Warning:  Invalid argument supplied for foreach() in C:\xampp\htdocs\testsite\testadmin\includes\classes\AdminRequestSanitizer.php on line 511

Here's the current code, with line 511 highlighted:

    private function filterMultiDimensional($parameterName, $parameterDefinition)
    {
        $requestPost = $_POST;
        [I]foreach ($requestPost[$parameterName] as $key => $value) {[/I]
            $hacked = $requestPost[$parameterName][$key];
            if (isset($parameterDefinition['params'][$parameterName])) {
                unset($requestPost[$parameterName][$key]);
                unset($_POST);
                $_POST[$parameterName] = $key;
                $type = $parameterDefinition['params'][$parameterName]['sanitizerType'];
                $params = isset($parameterDefinition['params'][$parameterName]['params']) ? $parameterDefinition['params'][$parameterName]['params'] : null;
                $newParameterDefinition = array('sanitizerType' => $type, 'params' => $params);
                $this->runSpecificSanitizer($parameterName, $newParameterDefinition);
                $newKey = $_POST[$parameterName];
                $requestPost[$parameterName][$newKey] = $hacked;
            }
            foreach ($hacked as $pkey => $pvalue) {
                if (isset($parameterDefinition['params'][$pkey])) {
                    unset($requestPost[$parameterName][$newKey][$pkey]);
                    unset($_POST);
                    $_POST[$pkey] = $pvalue;
                    $type = $parameterDefinition['params'][$pkey]['sanitizerType'];
                    $params = isset($parameterDefinition['params'][$pkey]['params']) ? $parameterDefinition['params'][$pkey]['params'] : null;
                    $newParameterDefinition = array('sanitizerType' => $type, 'params' => $params);
                    $this->runSpecificSanitizer($pkey, $newParameterDefinition);
                    $requestPost[$parameterName][$newKey][$pkey] = $_POST[$pkey];
                }
            }

        }
        $_POST = $requestPost;
    }

It looks like you'd want to change that to

    private function filterMultiDimensional($parameterName, $parameterDefinition)
    {
        $requestPost = $_POST;
       [B] if (isset ($requestPost[$parameterName])) {[/B]
            foreach ($requestPost[$parameterName] as $key => $value) {
                $hacked = $requestPost[$parameterName][$key];
                if (isset($parameterDefinition['params'][$parameterName])) {
                    unset($requestPost[$parameterName][$key]);
                    unset($_POST);
                    $_POST[$parameterName] = $key;
                    $type = $parameterDefinition['params'][$parameterName]['sanitizerType'];
                    $params = isset($parameterDefinition['params'][$parameterName]['params']) ? $parameterDefinition['params'][$parameterName]['params'] : null;
                    $newParameterDefinition = array('sanitizerType' => $type, 'params' => $params);
                    $this->runSpecificSanitizer($parameterName, $newParameterDefinition);
                    $newKey = $_POST[$parameterName];
                    $requestPost[$parameterName][$newKey] = $hacked;
                }
                foreach ($hacked as $pkey => $pvalue) {
                    if (isset($parameterDefinition['params'][$pkey])) {
                        unset($requestPost[$parameterName][$newKey][$pkey]);
                        unset($_POST);
                        $_POST[$pkey] = $pvalue;
                        $type = $parameterDefinition['params'][$pkey]['sanitizerType'];
                        $params = isset($parameterDefinition['params'][$pkey]['params']) ? $parameterDefinition['params'][$pkey]['params'] : null;
                        $newParameterDefinition = array('sanitizerType' => $type, 'params' => $params);
                        $this->runSpecificSanitizer($pkey, $newParameterDefinition);
                        $requestPost[$parameterName][$newKey][$pkey] = $_POST[$pkey];
                    }
                }

            }
[B]        }[/B]
        $_POST = $requestPost;
    }

@wilt, I just found the following warning in my /logs folder; it's issued when the edit_orders script is initially entered (i.e. no $_POST parameters).

Fix pushed here
https://github.com/zencart/zencart/pull/894/commits/3e07484bb9abc7f1dafe8cdd8916edf14a41984d

It turns out that the add-product handling from Edit Orders has some challenges fitting into the mold provided by the sanitizer, when attributes are involved.

If you look, for instance, at the demo product (Big Linked/Bug's Life ...) cPath=22&products_id=34. That product has multiple attributes, including one (the Gift Options checkboxes) that presents itself as an array. Here's a dump of the $_POST information coming back from an add of that product (pre-sanitization):

Array
(
    [securityToken] => 608c5b6efd7f704accd12713fa833510
    [id] => Array
        (
            [1] => Array
                (
                    [value] => 85
                    [type] => 2
                )

            [2] => Array
                (
                    [value] => 42
                    [type] => 0
                )

            [5] => Array
                (
                    [value] => 48
                    [type] => 0
                )

            [6] => Array
                (
                    [value] => 45
                    [type] => 0
                )

            [13] => Array
                (
                    [value] => Array
                        (
                            [63] => 63
                        )

                    [type] => 3
                )

            [10] => Array
                (
                    [value] => Here's a line of "text"
                    [type] => 1
                )

            [9] => Array
                (
                    [value] => 
                    [type] => 1
                )

            [11] => Array
                (
                    [value] => 
                    [type] => 1
                )

        )

    [add_product_categories_id] => 22
    [add_product_products_id] => 34
    [search] => 
    [step] => 4
)

So, the 'value' field of that attributes' list can be either a number, a collection of characters or an array! I'm not sure how to encode that currently; any guidance would be appreciated.

I believe I've found the way to "describe" those inputs to the sanitizer, adding the following to the Edit Orders sanitizer:

        'id' => array (
            'sanitizerType' => 'MULTI_DIMENSIONAL',
            'method' => 'post',
            'pages' => array ('edit_orders'),
            'params' => array (
                'id' => array ('sanitizerType' => 'CONVERT_INT'),
                'type' => array ('sanitizerType' => 'CONVERT_INT'),
                'value' => array ('sanitizerType' => 'PRODUCT_DESC_REGEX'),
            ),
            
        )

@wilt or @DrByte, would you review/comment as to whether the PRODUCT_DESC_REGEX is appropriate for this structure?

Hi,

grrrr.

I guess as a quick fix you could assign a NULL_ACTION to the value parameter as part of a MULTI_DIMENSIONAL sanitizer.

Fortunately you caught me at a point where I was preparing a PR for the sanitizer due to a different regression error.

It would be nice to have a generic sanitizer for this case, and I may work on that, but for now I will probably just look at doing an ATTRIBUTES_VALUE sanitizer.

Watch this space :)

lat9:

It turns out that the add-product handling from Edit Orders has some challenges fitting into the mold provided by the sanitizer, when attributes are involved.

If you look, for instance, at the demo product (Big Linked/Bug's Life ...) cPath=22&products_id=34. That product has multiple attributes, including one (the Gift Options checkboxes) that presents itself as an array. Here's a dump of the $_POST information coming back from an add of that product (pre-sanitization):

Array
(
[securityToken] => 608c5b6efd7f704accd12713fa833510
[id] => Array
(
[1] => Array
(
[value] => 85
[type] => 2
)

        [2] => Array
            (
                [value] => 42
                [type] => 0
            )

        [5] => Array
            (
                [value] => 48
                [type] => 0
            )

        [6] => Array
            (
                [value] => 45
                [type] => 0
            )

        [13] => Array
            (
                [value] => Array
                    (
                        [63] => 63
                    )

                [type] => 3
            )

        [10] => Array
            (
                [value] => Here's a line of "text"
                [type] => 1
            )

        [9] => Array
            (
                [value] => 
                [type] => 1
            )

        [11] => Array
            (
                [value] => 
                [type] => 1
            )

    )

[add_product_categories_id] => 22
[add_product_products_id] => 34
[search] => 
[step] => 4

)

> So, the 'value' field of that attributes' list can be either a number, a collection of characters or an array!  I'm not sure how to encode that currently; any guidance would be appreciated.

Hi

That does in fact seem like a cool solution, although DrByte and I were talking and think PRODUCT_NAME_DEEP_REGEX is better than PRODUCT_DESC_REGEX as it is less permissive.

lat9:

I believe I've found the way to "describe" those inputs to the sanitizer, adding the following to the Edit Orders sanitizer:

    'id' => array (
        'sanitizerType' => 'MULTI_DIMENSIONAL',
        'method' => 'post',
        'pages' => array ('edit_orders'),
        'params' => array (
            'id' => array ('sanitizerType' => 'CONVERT_INT'),
            'type' => array ('sanitizerType' => 'CONVERT_INT'),
            'value' => array ('sanitizerType' => 'PRODUCT_DESC_REGEX'),
        ),
        
    )

> @wilt or @DrByte, would you review/comment as to whether the PRODUCT_DESC_REGEX is appropriate for this structure?

HI

On the other hand we need to consider what the TEXT attribute was meant for.

Assume you have a T-Shirt shop where the customer can define the text that appears on the T-Shirt

Using PRODUCT_NAME_DEEP_REGEX means the while the customer could order a t-shirt with

using
PRODUCT_NAME_DEEP_REGEX as a sanitizer , would mean edit_orders would reject that,

So maybe PRODUCT_DESC_REGEX is a better option

wilt:

Hi

That does in fact seem like a cool solution, although Chris and I think PRODUCT_NAME_DEEP_REGEX is better than PRODUCT_DESC_REGEX as it is less permissive.

As I posted in the EO forum, that construct I posted worked with pre-ZC1.5.5a postings of the AdminSanitizer class, but fails with the ZC1.5.5a version.

I finally got the AdminRequestSanitizer to spit out some debug (shouldn't it have been as simple as calling the setDebug function with an argument of true?). It looks like part of the issue is that EO uses the variable named "id" (just like the storefront) to hold those added-products' attributes and the built-in 'id' definition is taking precedence:

May-13-2016 11:51:00
=================================

Incoming GET Request Array
(
    [page] => 1
    [oID] => 8421
    [action] => add_prdct
)


Incoming POST Request Array
(
    [securityToken] => cb8ec48cbfe04dbde8aeb3dd0a4f7b34
    [id] => Array
        (
            [35] => Array
                (
                    [value] => 2271
                    [type] => 0
                )

            [48] => Array
                (
                    [value] => 3244
                    [type] => 0
                )

            [43] => Array
                (
                    [value] => 2406
                    [type] => 0
                )

            [20] => Array
                (
                    [value] => 2259
                    [type] => 0
                )

            [44] => Array
                (
                    [value] => 
                    [type] => 1
                )

        )

    [add_product_categories_id] => 110
    [add_product_products_id] => 287
    [search] => 
    [step] => 4
)


Running Admin Sanitizers

PROCESSING SIMPLE_ALPHANUM_PLUS(GET) == action

[B]PROCESSING SIMPLE_ALPHANUM_PLUS(POST) == id[/B]

PROCESSING SIMPLE_ALPHANUM_PLUS(GET) == oID

PROCESSING SIMPLE_ALPHANUM_PLUS(GET) == page

PROCESSING STRICT_SANITIZE_VALUES == securityToken

PROCESSING STRICT_SANITIZE_VALUES == add_product_categories_id

PROCESSING STRICT_SANITIZE_VALUES == add_product_products_id

PROCESSING STRICT_SANITIZE_VALUES == search

PROCESSING STRICT_SANITIZE_VALUES == step

Outgoing GET Request Array
(
    [page] => 1
    [oID] => 8421
    [action] => add_prdct
)


Outgoing POST Request Array
(
    [securityToken] => cb8ec48cbfe04dbde8aeb3dd0a4f7b34
    [id] => Array
        (
            [35] => Array
            [48] => Array
            [43] => Array
            [20] => Array
            [44] => Array
        )

    [add_product_categories_id] => 110
    [add_product_products_id] => 287
    [search] => 
    [step] => 4
)

Note: I don't think it's a possibility to change that 'id' variable's name; there might be other non-EO code that's making use of that variable and I don't want to create a real train-wreck.

FWIW, editing /admin/includes/init_includes/init_sanitize.php and commenting out the "built-in" processing for the **id **parameter (lines 81 and 172) and using the construct that I posted above appears to produce the desired results.

Not knowing why the **id **parameter was added to the built-in list (twice, so it must be very important:P), I consider that change a work-around rather than a proper correction.

I'll take a work-around! :) OK will test tonight and let you know. Can you push to your repo?

This should work.

I note that if I add $sanitizer->setDebug(true); to the end of admin/includes/init_includes/init_sanitize.php I get Sanitize logs

lat9:

I finally got the AdminRequestSanitizer to spit out some debug (shouldn't it have been as simple as calling the setDebug function with an argument of true?).

Hi

Are you using the 'page' override that is available for $sanitizer->addComplexSanitization($group);

lat9:

FWIW, editing /admin/includes/init_includes/init_sanitize.php and commenting out the "built-in" processing for the **id **parameter (lines 81 and 172) and using the construct that I posted above appears to produce the desired results.

Not knowing why the **id **parameter was added to the built-in list (twice, so it must be very important:P), I consider that change a work-around rather than a proper correction.

swguy:

I'll take a work-around! :) OK will test tonight and let you know. Can you push to your repo?
I've pushed the update to the eo_sanitization.php file to account for the add-product path (including the corrected check). I have not (and won't unless it's absolutely needed) pushed the work-around.

wilt:

This should work.

If I add $sanitizer->setDebug(true); to the end of admin/includes/init_includes/init_sanitize.php I get Sanitize logs
When I added $eo_sanitizer->setDebug(true); to the end of admin/includes/extra_datafiles/eo_sanitization.php ... it doesn't.

lat9:

I've pushed the update to the eo_sanitization.php file to account for the add-product path (including the corrected check). I have not (and won't unless it's absolutely needed) pushed the work-around.

The work around (the change to admin/includes/init_includes/init_sanitize.php suggested in https://www.zen-cart.com/showthread.php?219747-AdminRequestSanitizer-Error-Log&p=1310878#post1310878) appears to not be needed. Your change to admin/includes/extra_datafiles/eo_sanitization.php works perfectly for the issues I was having.

Thanks for your hard work on this fix.

swguy:

The work around (the change to admin/includes/init_includes/init_sanitize.php suggested in https://www.zen-cart.com/showthread.php?219747-AdminRequestSanitizer-Error-Log&p=1310878#post1310878) appears to not be needed. Your change to admin/includes/extra_datafiles/eo_sanitization.php works perfectly for the issues I was having.

Thanks for your hard work on this fix.
swguy, you're right!:clap: When I remove those commented-out lines from my client's fresh 1.5.5a/EO4.1.5-beta2 installation ... adding a product with attributes works! Sounds like it might be time to package up EO v4.1.5 for release (more likely tomorrow).

Cool. Great news.