Make a minor docs improvement to NormalizerBase.php in order for Patchman to see it as a newer version than it was prior to SA-CORE-2019-003

Are you still using Drupal 8.6.9 or 8.5.10, or older versions?

What your hosting company did is patch your site for https://www.drupal.org/sa-core-2019-003, which they should only do if you're not yet on Drupal 8.6.10/8.5.11 or newer. They modified the official security patch and applied it to your site.

If you already are on a secure Drupal 8 release (by now other security releases have shipped, so 8.6.10/8.5.11 are no longer recommended), then your hosting company shouldn't have patched your site.

Looking forward to hear back from you!

If you're on Drupal 8.7.2, you're definitely already secure, and this is a problem caused by your hosting provider's use of this "Patchman" third party service!

I would like to be clear on this!

I too am hosted at A2 Hosting and am constantly receiving emails generated by the Patchman Program that have installed and running on my shared hosting account in my CPanel.

The program consistently sends me warnings about a vulnerability with the same files:
Input Validation vulnerability in Drupal
/home/MYACCOUNT/public_html/core/modules/serialization/src/Normalizer/NormalizerBase.php

Note: I also have two development versions in the same account and those paths are duplicated in the report!

I maintain my updates as soon as they appear (within a day or two) and updated to Drupal Version 8.7.4 just the other day, and yet received an email this morning stating the above vulnerability!

I believe the Patchman program may allow me to exempt this file, but I have concerns about whether the file indeed is in need of attention from its developer(s) or if it merely is being falsely flagged by Patchman!

Any advise?

One more for A2 Hosting and the email message regarding this "patch".

Am on Drupal Version 8.7.6 and all is upto date.

Where is patchman getting the patch from and why is this not being patched when updating core?

Fresh installs of Drupla 8 also at A2 hosting :: getting the same kind of Patchman messages:

It appears patches are available for application(s) installed in the following path(s):

Input Validation vulnerability in Drupal
/home/xxx/xxx/core/modules/serialization/src/Normalizer/NormalizerBase.php

Clearly D8 new installs are all up to date.

I don't understand that this issue is listed as Closed (fixed).

oh, A2 points to their patch scan process:
Click here to learn more about our perpetual security scans: https://www.a2hosting.com/kb/cpanel/advanced-features/patchman 

Anyone know the process for engaging with Patchman to fix this on their end?

Just a wild guess here, but I wonder if what's happening is due to a combination of:

1. Patchman having backported SA-CORE-2019-003 into changes to NormalizerBase.php (which is not one of the files changed by that SA, but maybe patchman felt a need to write a backport that modifies NormalizerBase for whatever reason).
2. Drupal not having changed NormalizerBase in any newer version since SA-CORE-2019-003 came out (last modification of that file was April 2018).

In other words, Patchman thinks that file was vulnerable prior to SA-CORE-2019-003 (which it wasn't), and that's the file that people still have, even with newer Drupal versions.

So maybe we can make patchman happy by simply committing some minor change to NormalizerBase.php so as to give it a newer timestamp and contents than what its state was prior to SA-CORE-2019-003?

Very interesting theory!

This patch applies to 9.0, 8.9, 8.8, 8.7. Could someone on A2 hosting try uploading a fresh copy of NormalizerBase.php that has had this patch applied and see if Patchman still modifies it?

It was honestly hard to find something worth changing. I felt like this code comment was a bit of a run-on sentence which made it a good choice to split.

Leaving as "Needs work" for now as I don't want the testbot to waste its time on this until we know it will address the patchman issue.

@greggles
This came today, attached is the file as a txt:

Hello,

We recently sent you an email regarding vulnerabilities detected on your domain(s) xx.xxnet hosted on az1-ls2.a2hosting.com. As promised in our previous email, we have gone ahead and applied patches to fix the following vulnerabilities:

Input Validation vulnerability in Drupal
/home/indust10/dev1/core/modules/serialization/src/Normalizer/NormalizerBase.php

Input Validation vulnerability in Drupal
...../core/modules/serialization/src/Normalizer/NormalizerBase.php

Click here to learn more about our perpetual security scans: https://www.a2hosting.com/kb/cpanel/advanced-features/patchman

Best Regards,

The A2 Hosting Support Team

I use Patchman, and this issue annoys the hell out of me, but I don't think there is anything we can do on the Drupal side, since it is caused by the inability for Patchman to add new files to a system.

This is the official explanation:

Author's note: The vulnerability for "drupal/core/modules/hal/src/Normalizer/FieldItemNormalizer.php" might also be detected for versions higher than 8.6.9. This occurs because Drupal developers shipped a security fix in a separate file, but Patchman is only able to modify files (to Patch them), not create new ones on your system. As a result, our only option was to back-port the fix into an existing application file. Regrettably, the file which hosts the solution may also exist in later Drupal versions not affected by the vulnerability. If your application runs on Drupal 8.6.10 or above, and you see this detection, it does not necessarily imply a vulnerability, but neither will a subsequent Patch have any negative impact.

@mr.baileys that's great research! Perhaps you could try out #12 and see if that stops the unnecessary patching?

With respect to @mr.baileys quote, I can confirm that this issue predates Drupal 8.6.10 by no less than 6 months!

I have been receiving emails from A2 Hosting's Patchman since sometime in 2018 every time I update Drupal core!

So just for some redundancy, here's what I got today!

Hello,

As part of our commitment to providing you with a secure hosting environment, we performed an automated scan of your domain(s) 'MY RECENTLY UPDATED SITES' hosted on az1-ls2.a2hosting.com

It appears patches are available for application(s) installed in the following path(s):

Input Validation vulnerability in Drupal
/home/...../html/core/modules/serialization/src/Normalizer/NormalizerBase.php

Input Validation vulnerability in Drupal
/home/...../html/core/modules/serialization/src/Normalizer/NormalizerBase.php

If you are working with a development partner, please forward this email on to them as they will be able to take care of the update for you. Otherwise, we will automatically apply the above patches within seven days.

Click here to learn more about our perpetual security scans: https://www.a2hosting.com/kb/cpanel/advanced-features/patchman

Best regards,

The A2 Hosting Support Team

Which of course will be followed up in a few days with a report that they have saved the day and patched the file!

IT GETS WORSE

Got this one today!

Hello,

We recently sent you an email regarding vulnerabilities detected on your domain(s) crew-list.industrycentral.net hosted on az1-ls2.a2hosting.com. As promised in our previous email, we have gone ahead and applied patches to fix the following vulnerabilities:

Code injection vulnerability in Drupal
/home/indust10/crew-list/html/core/includes/bootstrap.inc

Incorrect permissions vulnerability in Drupal
/home/indust10/crew-list/html/core/modules/jsonapi/src/Controller/EntityResource.php

Incorrect permissions vulnerability in Drupal
/home/indust10/crew-list/html/core/modules/jsonapi/src/Controller/FileUpload.php

CSRF vulnerability in Drupal
/home/indust10/crew-list/html/core/lib/Drupal/Core/Form/FormValidator.php

CSRF vulnerability in Drupal
/home/indust10/crew-list/html/core/lib/Drupal/Core/Form/FormBuilder.php

Click here to learn more about our perpetual security scans: https://www.a2hosting.com/kb/cpanel/advanced-features/patchman

Best Regards,

The A2 Hosting Support Team

I put a patch in #12 that might help fix this problem.

Please test it.

I raised this issue to A2 Hosting again as of Jun 20, 2020. There is now an open ticket for all hosted Drupal 8 sites. They also sent the following:
——-
Greetings!

Thank you for your report.

Patchman patches can be disabled in the cPanel interface. I've provided an article below on how to do this. We've also reported this bug to our Operations team for further investigation to see if they can get the issue with the false-positive emails resolved. We do see a couple of other reports complaining about Patchman catching the "NormalizerBase.php" file with Drupal 8 specifically. We will monitor their investigation and provide you with updates as they become available.

How to access and manage Patchman
https://www.a2hosting.com/kb/cpanel/advanced-features/patchman#Managing-...

Please do not hesitate to ask if you have any other questions or concerns.
Best Regards,

Hello,

My name is Jelmer Verkleij, and I am the Chief Technology Officer at Patchman. As I understand this may be somewhat confusing, I would like to add additional clarification, in addition to the comments that have in fact already been quoted here from support cases we handled in the past.

Specifically, the messages from effulgentsia and mr.baileys already explain the core element of this. I hope I can shed some additional light on the background.

One of Patchman's key features is that we backport official security fixes to older versions of the same software, so that websites can be secured without requiring an immediate full update. We strongly believe that an update is always the best path forward for ongoing security, but this approach gives website owners some time (and breathing room) to prepare for such an update, as we all know that depending on configuration, themes, plugins etc, that may not be trivial to complete successfully.

When we backport a security fix for older (Drupal) versions, we can only patch existing files, not create entirely new ones. That is to say, we explicitly do not want to add that functionality to our product. We have a responsibility to create a secure product with minimal risk to all the websites we are hired to protect, and every additional feature we add to our own product risks creating more bugs and/or attack surface for malicious actors. That care is not done instead of, but in addition to rigorous testing we do of our product anyway. Our product is designed specifically to only be able to modify files which are marked by our own signature set as being vulnerable - that means we've intentionally limited our software to not be able to modify random files on your websites, let alone create or delete them. All this to say that this is a self-imposed restriction.

In the vast majority of cases, this doesn't actually matter. Many vulnerabilities don't actually require new files to be added - new code to pre-existing files is far more common. However, there are exceptions, and this specific advisory is one of them. And that means we are forced to use a workaround.

That workaround is essentially to ship code in another file than the new file - in this case, NormalizerBase.php. Again, this by itself is usually not an issue, if the file we pick happens to be changed in a future version, because then it is different from the state of that file we associate with vulnerable applications.

Unfortunately, this is a very specific case where that doesn't hold - the file went unchanged long after the vulnerability got fixed in the official Drupal versions. That results in Patchman incorrectly flagging new versions as vulnerable. I am very aware that this is confusing, annoying and maybe even alarming - we are telling you that something is vulnerable when in fact it really isn't. However, we have put a lot of care into our patches to make sure that executing them even on these newer versions doesn't actually break anything - it is a benign no-op, as it were - and the few cases where this leads to miscommunication are far outweighed by the hundreds of thousands of Drupal websites we managed to secure by shipping this patch to our customers.

Naturally, there is a key takeaway from this for us: using NormalizerBase.php was probably not the best choice, and whenever a new case like this pops up in future security releases, we will continue to try and find other files which may be more prone to upstream changes than this one (although of course we don't want to stick a change just anywhere randomly in the application, as that has other clear downsides).

None of this is going to change the emails that were sent out, nor are we going to change the way we flag this specific vulnerability going forward. However, there are two additional notes I can offer as potential remediation:

• As correctly pointed out by greggles, any official change to this file will stop us from flagging the file as vulnerable; we only mark something as vulnerable if it is identical to our signatures, so in theory even a whitespace change would stop this from happening. It will of course only work for newer versions, not for any of the already released non-vulnerable versions.
• As long as the file on the server doesn't actually change, we will not flag a file as being newly vulnerable - even if a write occurs in the meantime (tracked in our case by the ctime property). In other words, simply updating your Drupal to another version that has the exact same NormalizerBase.php file should not cause a new detection and/or resulting email, and in fact I have not been able to reproduce that. There are probably two exceptions to this:
  • It is different if you install your Drupal version in a new directory (where it is considered a new file to Patchman) and subsequently move it.
  • If Patchman patched the file after the previous detection, updating it would overwrite that file with the once-again 'vulnerable' state - that would change the file, and would create a new detection.

  I am interested to know if either of these flows applise to e.g. bas123 - if they don't, I definitely want to investigate how a regular upgrade would cause these messages to occur. As it is definitely not my intention to hijack this topic and turn it into a Patchman support conversation, feel free to send me an email personally at jelmer.verkleij@patchman.co with more information so I can look into this for you.

I hope I was able to shed some light on the background of this, and while it may not immediately alleviate the problems you are having, I hope you understand that our design decisions are always made with maximum website security for Drupal site owners in mind.

Best regards,

Jelmer Verkleij
Chief Technology Officer
Patchman B.V.

Thank you for the confirmation and explanation, Jelmer!

I think that answers the question from #12, so I'm RTBCing that patch as something worth getting into 9.1. I think it's also harmless enough and worthwhile to cherry pick into 9.0 as well, but it needs to get into 9.1 first.

Updated the issue summary for committers and people coming to this issue in the future.

New vulnerabilities are being reported by Patchman as of the latest Drupal v 8.8.10 and Open Social v 8.x-9.2 updates

I do not know how or where these need reporting so perhaps this can be disseminated from here!

It appears patches are available for application(s) installed in the following path(s):

Information disclosure vulnerability in Drupal
/home/xxx/html/core/modules/file/src/Element/ManagedFile.php

XSS vulnerability in Drupal
/home/xxx/html/core/modules/filter/src/Plugin/Filter/FilterCaption.php

XSS vulnerability in Drupal
/home/xxx/html/core/modules/ckeditor/js/plugins/drupalimagecaption/plugin.js

XSS vulnerability in Drupal
/home/xxx/html/core/modules/ckeditor/js/plugins/drupalimagecaption/plugin.es6.js

Incorrect permissions vulnerability in Drupal
/home/xxx/html/core/modules/workspaces/src/WorkspaceManager.php

XSS vulnerability in Drupal
/home/xxx/html/core/misc/autocomplete.js

XSS vulnerability in Drupal
/home/xxx/html/core/misc/autocomplete.es6.js

XSS vulnerability in Drupal
/home/xxx/html/core/misc/ajax.js

XSS vulnerability in Drupal
/home/xxx/html/core/misc/ajax.es6.js

If you are working with a development partner, please forward this email on to them as they will be able to take care of the update for you. Otherwise, we will automatically apply the above patches within seven days.

HIding 2 unnecessary files from the issue summary. This remains RTBC by effulgentsia from #21 after insight from the Patchman CTO in #20.

Re-uploading 3054510-change-file-to-avoid-errant-patching.patch so it's get tested ewvery 2 days against 9.2.x-dev.

I don't think the fails are from the patch in #26.

The core automated testing page says there are some fails in core on its own.

Back to RTBC based on effulgentsia in #21.

Also adjusting credit a smidge.

longwave suggested in slack that these random fails are likely just less reliable tests.

Back to RTBC based on effulgentsia in #21.

Committed and pushed 6366c55572 to 9.2.x and 373528eda0 to 9.1.x. Thanks!

Hopefully in the future patchman chooses a better file / way to work out if a site is vulnerable. I'm not a huge fan of this patch and hopefully committing this will not set a precedent for these types of fixes in the future - however given this solves users current issues going to commit and ask a release manager about backport to 8.9.x

Thanks so much, @alexpott.

From the perspective of the Security Team members who deal with support requests related to this, it would be great to backport to 8.9.x. I'm going to tentatively move to needs review on that branch, but if a decision is made not to backport it can of course be moved back to fixed.

I also agree that I hope we won't have to make a habit of making commits like this.

This is indeed a very annoying thing. I'm setting the patch to retest on 8.9.x and if it's green (why not, right) I think this is RTBC

Discussed with @catch we agreed to backport this to 8.9.x

Thank you alexpott, catch and everyone who helped with this issue. I'm glad we were able to come up with a solution to prevent this confusing message from the patchman software.