Problem/Motivation

PHPCS 2.8.1 was released with a security fix: https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/2.8.1
This does not directly affect us because we don't use one of the vulnerable sniffs. The diff report might be used by the testbots, but there is not much damage an attacker could do in that sandbox. That is also the reason why this issue is public, because there is no direct vulnerability.

Security fixes are still critical.

Proposed resolution

Update PHPCS alone for the security fix, Coder can be updated later.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi created an issue. See original summary.

klausi’s picture

Status: Active » Needs review
FileSize
2.03 KB

Patch.

There are some unrelated composer.lock changes in there, probably because I use a newer Composer version? They don't hurt though and since composer.lock is a machine generated file I think we should take it as is.

klausi’s picture

Issue tags: +Security

Tagging.

klausi’s picture

Patch created with

composer update squizlabs/php_codesniffer
pwolanin’s picture

+++ b/composer.lock
@@ -1099,12 +1099,12 @@
+                "url": "https://api.github.com/repos/php-fig/log/zipball/1.0.0",
+                "reference": "1.0.0",

odd?

Can you document how you did the composer.lock update?

klausi’s picture

Category: Task » Bug report

classifying as a bug so that we don't forgot to cherry-pick to all supported 8.x branches.

klausi’s picture

@pwolanin, sure:
Patch created with

composer update squizlabs/php_codesniffer

As I said: a newer composer version simplifies to auto-generated content of composer.lock.

pwolanin’s picture

Status: Needs review » Needs work

I don't get the fig change locally

I get:

diff --git a/composer.lock b/composer.lock
index a8e3057..482cd43 100644
--- a/composer.lock
+++ b/composer.lock
@@ -4141,16 +4141,16 @@
         },
         {
             "name": "squizlabs/php_codesniffer",
-            "version": "2.7.1",
+            "version": "2.8.1",
             "source": {
                 "type": "git",
                 "url": "https://github.com/squizlabs/PHP_CodeSniffer.git",
-                "reference": "9b324f3a1132459a7274a0ace2e1b766ba80930f"
+                "reference": "d7cf0d894e8aa4c73712ee4a331cc1eaa37cdc7d"
             },
             "dist": {
                 "type": "zip",
-                "url": "https://api.github.com/repos/squizlabs/PHP_CodeSniffer/zipball/9b324f3a1132459a7274a0ace2e1b766ba80930f",
-                "reference": "9b324f3a1132459a7274a0ace2e1b766ba80930f",
+                "url": "https://api.github.com/repos/squizlabs/PHP_CodeSniffer/zipball/d7cf0d894e8aa4c73712ee4a331cc1eaa37cdc7d",
+                "reference": "d7cf0d894e8aa4c73712ee4a331cc1eaa37cdc7d",
                 "shasum": ""
             },
             "require": {
@@ -4215,7 +4215,7 @@
                 "phpcs",
                 "standards"
             ],
-            "time": "2016-11-30T04:02:31+00:00"
+            "time": "2017-03-01T22:17:45+00:00"
         },
         {
             "name": "symfony/browser-kit",
pwolanin’s picture

Status: Needs work » Needs review
FileSize
1.33 KB

Note that klausi and I both updated to composer 1.3.2 , but here's what I get.

klausi’s picture

Status: Needs review » Reviewed & tested by the community

Indeed, removed my vendor dir and performed a fresh composer install and then the update. That gets me the same result, sorry for the confusion with the first patch.

Looks good!

Mixologic’s picture

FYI: the installed.json from the test now shows up in the artifacts: https://dispatcher.drupalci.org/job/drupal_patches/4131/artifact/jenkins...

so this confirms that 2.8.1 is getting installed, and the patch in #2 is setting php-fig log to : fe0936ee26643249e916849d48e3a51d5f5e278b which also corresponds to 1.0.0

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed b0af9e9 to 8.4.x and 214457a to 8.3.x. Thanks!

PHPCS has not been a dependency in any actually released version of Drupal 8 - it is also only a dev dependency and therefore is covered by the earlier PSA about not deploying these.

  • alexpott committed b0af9e9 on 8.4.x
    Issue #2857245 by klausi, pwolanin: Security update for PHPCS
    

  • alexpott committed 214457a on 8.3.x
    Issue #2857245 by klausi, pwolanin: Security update for PHPCS
    
    (cherry...
klausi’s picture

Thanks, next Coder update: #2857714: Upgrade Coder to 8.2.11

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.