Security Scan reports SUPEE-11219 as failed but it's installed

[C4rter]

Alright, if that are weak passwords I can use them without an error in the shop in question.

As long as I reach the minimum char limit of 7 it's going through with 123ABCD as password.

I had a look into the validate function in app\code\core\Mage\Customer\Model\Customer.php

The only password validation I found is this:

$password = $this->getPassword();
if (!$this->getId() && !Zend_Validate::is($password , 'NotEmpty')) {
    $errors[] = Mage::helper('customer')->__('The password cannot be empty.');
}
$minPasswordLength = $this->getMinPasswordLength();
if (strlen($password) && !Zend_Validate::is($password, 'StringLength', array($minPasswordLength))) {
    $errors[] = Mage::helper('customer')
        ->__('The minimum password length is %s', $minPasswordLength);
}
if (strlen($password) && !Zend_Validate::is($password, 'StringLength', array('max' => self::MAXIMUM_PASSWORD_LENGTH))) {
    $errors[] = Mage::helper('customer')
        ->__('Please enter a password with at most %s characters.', self::MAXIMUM_PASSWORD_LENGTH);
}
$confirmation = $this->getPasswordConfirmation();
if ($password != $confirmation) {
    $errors[] = Mage::helper('customer')->__('Please make sure your passwords match.');
}

I don't see any weak password validation. And that's in the 1.9.4.3 Core Version.

Do you know where the weak password check is located (if there is any)?

It was supposed to be fixed in SUPEE-11155

https://magento.com/security/patches/supee-11155

"PRODSECBUG-2331: Weak password requirements when registering an account - CVE-2019-7918"

But when I look at the patch file, I can't see anything related either.

There was a change for Admin User Password in the latest patch, SUPEE-11219

That adds additional password checks to app/code/core/Mage/Admin/Model/User.php

        if ($this->hasNewPassword()) {
            $password = $this->getNewPassword();
        } elseif ($this->hasPassword()) {
            $password = $this->getPassword();
        }
        if (isset($password)) {
            $minAdminPasswordLength = $this->getMinAdminPasswordLength();
            if (Mage::helper('core/string')->strlen($password) < $minAdminPasswordLength) {
                $errors[] = Mage::helper('adminhtml')
                    ->__('Password must be at least of %d characters.', $minAdminPasswordLength);
            }

            if (!preg_match('/[a-z]/iu', $password) || !preg_match('/[0-9]/u', $password)) {
                $errors[] = Mage::helper('adminhtml')
                    ->__('Password must include both numeric and alphabetic characters.');
            }

            if ($this->hasPasswordConfirmation() && $password != $this->getPasswordConfirmation()) {
                $errors[] = Mage::helper('adminhtml')->__('Password confirmation must be same as password.');
            }

But that is not in place for Store customers, just admin accounts.

I added this part to the store customer validation, but that doesn't do it:

        if (!preg_match('/[a-z]/iu', $password) || !preg_match('/[0-9]/u', $password)) {
            $errors[] = Mage::helper('adminhtml')
                ->__('Password must include both numeric and alphabetic characters.');
        }

ABC1234 is still going through because there are characters and numbers.

What is Magento doing? It's not fixed at all!

[C4rter]

Do you know a way to check for weak passwords? We could then make our own change and check, if that's indeed what Magento is checking.

Or do you even have the check in your core code?

[wadeo]

I have found in the past with a past SUPEE patch was still showing up as being not applied. Then maybe a week or so later the scanner may have been updated and showed as being resolved.  MageReport is showing that this has been resolved so that is what I'm going to roll with. I will be monitoring this thread for any new information that comes up regarding the possible false positive for SUPEE-11219 - Failed

So far I have patched/upgraded 2 sites that are both reporting the same SUPEE-11219 - Failed. I will be working on several more this afternoon, hopefully, they can get this scanner working properly soon and fix all this false positive junk. If it is a vulnerability then tell us xxx file is present or line xxx on file xxx.php has an issue. 

[C4rter]

It started showing up a few days or even weeks after the patch was released.

I think that means that they added a specific check intended for this patch.

But the error does not match with the patch number. That's were it already starts to be really weird.

I think, Magento thinks they fixed something, which they in fact did not (weak passwords). And now they even added a check to test what they think they fixed. That will never work without an additional patch.

MageReport says everything is okay, but what exactly do they check to verify that? We don't no what Magento is checking, nor what MageReport is checking.

I will try to add a weak password regex check in the next few days and let Magento Security Check run against it to verify if they really try to register a customer with a weak password.

Who can we contact from Magento that can help us here?

[cosmo]

There is another validation function at js/prototype/validate.js

I have seen that it validates both the password creation in new account and in an existing account.

   ['validate-password', 'Please enter 8 or more characters. No clean leading or trailing spaces. Password should contain at least one digit, one lower case, one upper case and at least 8 from the mentioned characters', function(v, elm) {
                var pass=v.strip(); /*strip leading and trailing spaces*/
                var reMin = new RegExp(/^min-pass-length-[0-9]+$/);
                var minLength = 7;
                $w(elm.className).each(function(name, index) {
                    if (name.match(reMin)) {
                        minLength = name.split('-')[3];
                    }
                });
                return (!(v.length > 0 && v.length < minLength) && v.length == pass.length);
            }],
    ['validate-admin-password', 'Please enter 7 or more characters. Password should contain both numeric and alphabetic characters.', function(v, elm) {
                var pass=v.strip();
                if (0 == pass.length) {
                    return true;
                }
                if (!(/[a-z]/i.test(v)) || !(/[0-9]/.test(v))) {
                    return false;
                }
                var reMin = new RegExp(/^min-admin-pass-length-[0-9]+$/);
                var minLength = 7;
                $w(elm.className).each(function(name, index) {
                    if (name.match(reMin)) {
                        minLength = name.split('-')[4];
                    }
                });
                return !(pass.length < minLength);
            }],
    ['validate-cpassword', 'Please make sure your passwords match.', function(v) {
                var conf = $('confirmation') ? $('confirmation') : $$('.validate-cpassword')[0];
                var pass = false;
                if ($('password')) {
                    pass = $('password');
                }
                var passwordElements = $$('.validate-password');
                for (var i = 0; i < passwordElements.size(); i++) {
                    var passwordElement = passwordElements[i];
                    if (passwordElement.up('form').id == conf.up('form').id) {
                        pass = passwordElement;
                    }
                }
                if ($$('.validate-admin-password').size()) {
                    pass = $$('.validate-admin-password')[0];
                }
                return (pass.value == conf.value);
            }],
    ['validate-both-passwords', 'Please make sure your passwords match.', function(v, input) {
                var dependentInput = $(input.form[input.name == 'password' ? 'confirmation' : 'password']),
                    isEqualValues  = input.value == dependentInput.value;

                if (isEqualValues && dependentInput.hasClassName('validation-failed')) {
                    Validation.test(this.className, dependentInput);
                }

                return dependentInput.value == '' || isEqualValues;
            }],

If you change number (7) with another higher number all the passwords that have 7 characters (regardless of what they are) are automatically not valid.
Whatsoever my validate.js is in accordance with the files posted on Github for version 1.9.4.3.

---

[C4rter]

That's interesting. The file is originally called js/prototype/validation.js and not "js/prototype/validate.js", so you seem to have a different file altogether.

The part you posted is missing from the archive I've downloaded from Magento with version 1.9.4.3 and it's missing in the patch files.

Downloaded from here: https://magento.com/tech-resources/download

What's the source on Github for this version?

That's what it looks like in the sources directly from Magento:

    ['validate-password', 'Please enter more characters or clean leading or trailing spaces.', function(v, elm) {
                var pass=v.strip(); /*strip leading and trailing spaces*/
                var reMin = new RegExp(/^min-pass-length-[0-9]+$/);
                var minLength = 7;
                $w(elm.className).each(function(name, index) {
                    if (name.match(reMin)) {
                        minLength = name.split('-')[3];
                    }
                });
                return (!(v.length > 0 && v.length < minLength) && v.length == pass.length);
            }],
    ['validate-admin-password', 'Please enter more characters. Password should contain both numeric and alphabetic characters.', function(v, elm) {
                var pass=v.strip();
                if (0 == pass.length) {
                    return true;
                }
                if (!(/[a-z]/i.test(v)) || !(/[0-9]/.test(v))) {
                    return false;
                }
                var reMin = new RegExp(/^min-admin-pass-length-[0-9]+$/);
                var minLength = 7;
                $w(elm.className).each(function(name, index) {
                    if (name.match(reMin)) {
                        minLength = name.split('-')[4];
                    }
                });
                return !(pass.length < minLength);
            }],
    ['validate-cpassword', 'Please make sure your passwords match.', function(v) {
                var conf = $('confirmation') ? $('confirmation') : $$('.validate-cpassword')[0];
                var pass = false;
                if ($('password')) {
                    pass = $('password');
                }
                var passwordElements = $$('.validate-password');
                for (var i = 0; i < passwordElements.size(); i++) {
                    var passwordElement = passwordElements[i];
                    if (passwordElement.up('form').id == conf.up('form').id) {
                        pass = passwordElement;
                    }
                }
                if ($$('.validate-admin-password').size()) {
                    pass = $$('.validate-admin-password')[0];
                }
                return (pass.value == conf.value);
            }],
    ['validate-both-passwords', 'Please make sure your passwords match.', function(v, input) {
                var dependentInput = $(input.form[input.name == 'password' ? 'confirmation' : 'password']),
                    isEqualValues  = input.value == dependentInput.value;

                if (isEqualValues && dependentInput.hasClassName('validation-failed')) {
                    Validation.test(this.className, dependentInput);
                }

                return dependentInput.value == '' || isEqualValues;
            }],

Also, it's usually not enough to have this validation only in JS files. It should always be duplicated into the PHP code.

[C4rter]

Just received the new scan result and the warning is gone now. Maybe they fixed it or just removed it.

[cosmo]

I am sorry, it is my mistaken typing, you are correct it is "validation.js"
Github source is:
https://github.com/OpenMage/magento-mirror/tree/99b772f94de693228a787d69254911e060934cf1

At the corresponding path.
Also as of today Security Scan is failing only SUPEE-11155, at the same time both Magereport and Magentary see it as patched (ver. 1.9.3.4). 

[C4rter]

I can't find it via your link on GitHub either. That's the file when being opened with your link:

https://github.com/OpenMage/magento-mirror/blob/99b772f94de693228a787d69254911e060934cf1/js/prototyp...

Your link points to the latests commit "Updated to Magento 1.9.4.3" (99b772f94de693228a787d69254911e060934cf1), which should be correct. I just navigated to the file in this context.

This is the file under the specific 1.9.4.3 tag in the repository:

https://github.com/OpenMage/magento-mirror/blob/1.9.4.3/js/prototype/validation.js

I guess it's the same.

Both do not contain the additional password validation, just the basic validation. Can you send me a specific link to the file version that shows this?

[Flipmedia]

Patched M 1.9.4.1 instance with 11219, this instance had SUPEE 11155 applied in June. As follows:-

2019-06-28 15:34:32 UTC | SUPEE-11155_CE_1941 | CE_1.9.4.1 | v1 | 8038bdd8b74be213c7d0d2d554482a7f65457fe8 | Fri Jun 14 21:16:40 2019 +0000 | 3e5a308a990e31e012794982488c882347acc47a..HEAD

2019-11-07 22:42:27 UTC | SUPEE-11219_CE_1941 | CE_1.9.4.1 | v1 | 8a75939769eb82c1959e329be5b2486847208d60 | Fri Oct 4 21:26:43 2019 +0000 | 8038bdd8b74be213c7d0d2d554482a7f65457fe8..HEAD

I was greeted by the following warnings in the weekly security scan:-

SUPEE-11155 - Failed.
WYSIWYG editor stored XSS found (PRODSECBUG-2246) response body contains unexpected 'html += option("flash", "object");'

SUPEE-11219 - Failed.
Weak password requirements found (PRODSECBUG-2331) response body contains unexpected 'Please enter 6 or more characters'

"PRODSECBUG-2331" is not part of the 11219 patch, but is part of the 11155 patch...

This instance is a clean 1.9.4.1 with only these 2 patches installed.  Prior to installing 11219 the version with installed 11155 cleared many prior scans.

Magereport shows no issues with site, confirming both patches installed.

On reviewing commit dates I can see that a V4 version of 11155 was added to resolve issues with sites running PHP 7.2, this version was provided after I installed 11155.

I have now reverted both patches, and applied 11155 v4 and then 111219 on a testing instance.

When applying 11155 V4 I was alerted to issues with js/tiny_mce/plugins/media, this was resolved by refreshing this area from a clean 1.9.4.1 build.

The instance as been subscribed to security scan and I will report the result.