FriendsOfPHP/security-advisories

False positive on drupal/search_api_solr

Pton opened this issue · 15 comments

Pton commented

The Drupal module Search API Solr Search gets flagged by the security checker, because of the following issue: https://www.drupal.org/sa-contrib-2018-065. From what I can gather this issue only applies to Drupal 7 installations (correct me if I'm wrong here).

I'm running Drupal 8 with the latest 8.x-1.x (1.2.0) version of the Search APi Solr Search module. I would expect the security checker not to flag this installation as vulnerable.

This could be fixed by adjusting sa-contrib-2018-065.yaml to only apply to Drupal 7. In this case such could be achieved by specifying the branch containing the security issue, which would be 7.x-1.x in this case. But from what I can gather from #366 (comment) this won't be possible.

What would be a solution to this issue?

stof commented

this is related to the discussion happening in Roave/SecurityAdvisories#54

The root cause of the issue here is that the Drupal ecosystem has actually 2 different repositories (one for drupal 7 and one for drupal 8), which can use the same version numbers for packages while the code is not the same (due to their 8.x-1.x vs 7.x-1.x internal versioning).

stof commented

@Pton can you give more details about the tool you used to check that ? Was it the online checker at https://security.symfony.com/, the local check using the Symfony CLI client (https://symfony.com/download) or the composer conflict rules of roave/security-advisories ?

Pton commented

I'm using v5.0.2 of the sensiolabs/security-checker

stof commented

OK, so that means using the https://security.symfony.com/ API

@Pton I tested "drupal/search_api_solr:^1" with security-checker (from master branch) and can't reproduce the issue.

@Pton my bad (i got 1.x-dev when I try with ^1). I can reproduce the issue with composer require "drupal/search_api_solr:1.2.0"

Symfony Security Check Report
=============================

1 packages have known vulnerabilities.

drupal/search_api_solr (1.2.0)
------------------------------

 * [CVE-NONE-0001][]: Search API Solr Search - Moderately critical - Access bypass - SA-CONTRIB-2018-065

[CVE-NONE-0001]: https://www.drupal.org/sa-contrib-2018-065

Note that this checker can only detect vulnerabilities that are referenced in the SensioLabs security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.

This is weird considering drupal/entity excludes fine.

@stof probably because of drupal/entity version. The 8.x version is 1.0-rc1 and the 7.x SA is only between 1.0.0 and 1.9.0

Pton commented

I checked our CI script which runs the security checker. We're utilize the --end-point option as such:
vendor/bin/security-checker security:check composer.lock --verbose --end-point=http://security.sensiolabs.org/check_lock.
I'm not sure if that's anything different than the https://security.symfony.com/ API or how this would change the issue I'm raising here.

The http://security.sensiolabs.org/check_lock endpoint will be removed soon, you must not use it anymore.

stof commented

@fabpot according to Roave/SecurityAdvisories#54 (comment) the new endpoint is also affected by the fact that the Drupal ecosystem reuses the same version for composer packages in the 7 and 8 repositories (while they are not affected the same by advisories).
So I suggest reverting #371.

Two solutions here: reverting #371 or filtering Drupal advisories until we find a solution to make it work.

stof commented

Well, filtering them out would have to be done in all places using that DB. that would make them useless in the DB in my opinion. See Roave/SecurityAdvisories#54 (comment) for my argument. Composer relies on name + version to identify packages. This repo should be able to do the same, which rules out the drupal-contrib ecosystem which breaks that assumption.

#371 reverted now

Pton commented

Although I'm disappointed the drupal contrib modules won't be checked by the security checker. I'm happy the builds can run again without false positives.

Thank you everyone for jumping on this so quickly and fixing the issue at hand ❤️