Prestashop: Duplicate address in checkout Address step on address edition

[DelecroixQuentin]

Describe the bug

With the security fix apply (tested on 1.7.2.3, 1.7.4.3, 1.7.5.1, 1.7.6.2, 1.7.6.4), when editing an address from the "Address" step of the checkout, a new address will be created instead of editing the current one.

From the "Address" page of the customer account, this works well.

The problem is that the Tools::getValue('id_address') (introduced in this commit) can't read an address id in the $_GET or $_POST (see screenshot)

Expected behavior

Edit the address instead of creating a new one when the customer wants to edit his address

Steps to Reproduce

Steps to reproduce the behavior:

1. Login, if you don't have an address, create it
2. Add product in cart
3. Process checkout
4. On the "Address" step, edit your address and save
5. Back to the "Address" step
6. See error

Before:

After:

Additional information

• PrestaShop version: tested on 1.7.2.3, 1.7.4.3, 1.7.5.1, 1.7.6.2, 1.7.6.4
• PHP version: 7.2.21

[nand2]

Edit: I am not running a clean 1.7.6.4 installation, so the bug I am describing may not happen in a clean installation.

This pull request does not seem (not tested your patch, assumed) to fix the case where you edit your address in your profile (not in the checkout process).
How to reproduce : Go to your profile, edit an address. Put numbers in the firstname field in order to trigger a validation error. Then remove the number, and save. It will create a duplicate address. Reason: The url for the POST is set to /adresse?id_address=0

I would even say that part of the fix for CVE-2020-5250 in classes/form/CustomerAddressFormatter.php :

        $format = [
-            'id_address' => (new FormField())
-                ->setName('id_address')
-                ->setType('hidden'),

for the purpose of preventing editing other's addresses was unnecessary because the id_address was already validated (cf CustomerAddressPersister.php)

private function authorizeChange(Address $address, $token)
{
    if ($address->id_customer && (int)$address->id_customer !== (int)$this->customer->id) {
        // Can't touch anybody else's address
        return false;
    }
    [...]
}

and I was not able to reproduce the problem where one could overwrite the address of someone else.

That being said, on submission, the id_address should only be given once, and it was given twice here (as a GET and a POST parameter). So it is good it was removed as a POST parameter, but then it should be always be there as a GET paramater, which is not the case after a validation error.

And I am reaching here the limits of my Prestashop knowledge :-) I can't see why it is not filled again after a validation error without going deep into the code... Anyone can see why? Thanks!

[DelecroixQuentin]

Nice catch !
A possible fix (tested only for this case), in classes/form/CustomerAddressForm class, function getTemplateVariables(), change this

return array(
            'id_address' => (isset($this->address->id)) ? $this->address->id : 0,
            'action' => $this->action,
            'errors' => $this->getErrors(),
            'formFields' => $formFields,
        );

to this

return array(
            'id_address' => (isset($this->address->id)) ? $this->address->id : Tools::getValue('id_address', 0),
            'action' => $this->action,
            'errors' => $this->getErrors(),
            'formFields' => $formFields,
        );

[nand2]

Thanks, I confirm this fix the bug I described.

[Progi1984]

Fixed by #18073

[DelecroixQuentin]

Hey @Progi1984,
Did you notice this comment ?
There are another duplication address issue but this one is on customer address page.
Should we create another issue for this bug?

Thanks !

[khouloudbelguith]

Hi @DelecroixQuentin, @nand2,

Issue created here: https://github.com/PrestaShop/PrestaShop/issues/18100

Thanks!

[Progi1984]

Sorry ! Not seen the comment. Thanks @khouloudbelguith

[colinegin]

I reopen this issue as we have to fix it for 1765.

[colinegin]

Fixed by: #18103