Exclude packages from security review

[danepowell]

There are several instances where you might legitimately want to include a package with a security advisory in your project. Quite often, upgrading to a newer secure version of a package may be difficult or impossible, and the security vulnerability may be low criticality or not affect your project at all. You might also want to patch the insecure version of the module (using something like https://github.com/cweagans/composer-patches) rather than upgrading to a new version.

Unfortunately there's no way to allow these kinds of exceptions with SecurityAdvisories, and no workaround that I know of.

[Ocramius]

There are several instances where you might legitimately want to include a package with a security advisory in your project.

I can't really think of any. Self-patching is a solution that requires disabling tools that detect issues anyway. If you build workarounds, then the tooling won't be able to know anyway.

Quite often, upgrading to a newer secure version of a package may be difficult or impossible, and the security vulnerability may be low criticality or not affect your project at all.

That is true indeed, but that's also why this package acts at composer update time, and not at composer install time. Any composer update is a potentially risky operation if proper e2e testing isn't in place.

You might also want to patch the insecure version of the module (using something like https://github.com/cweagans/composer-patches) rather than upgrading to a new version.

This can be done in a way that is compatible with this module by using "replace": "package/name" or "provides": {"package/name": "1.0.99999"}. You are responsible for making sure your replacement is compatible though.

Unfortunately there's no way to allow these kinds of exceptions with SecurityAdvisories, and no workaround that I know of.

Adding exceptions is not something you'd do in security-sensitive contexts. If your package provides a patch before the actual vendor, then do use the "replace" or "provides" tricks described above.

[josefsabl]

Well, there's this: https://security.snyk.io/vuln/SNYK-PHP-FIREBASEPHPJWT-2434829 And this: https://github.com/tuupola/slim-jwt-auth/issues/217

I was looking for a way to "whitelist" a vulnerable package to find this thread. Now I need to remove security advisories altogether 😥.

[Ocramius]

TBH, sounds like you have a security issue, and you need to help with the affected library upgrade, or maintain your own fork of tuupola/slim-jwt-auth until upstream gets a stable release.

You are just shooting at the messenger here.

[josefsabl]

I didn't want to shoot or anything. I just wanted to showcase that this might be useful. Aforementioned vulnerability can be overcome with configuration which we checked so the vulnerability in fact is not there.

[Ocramius]

This library limits itself to declaring which ranges are not considered safe for installation: whether a CVE depends on misconfiguration or specific setup is not something that can be documented here.

If a library has a "security issue around de-serialization", yet you never deserialize stuff in your project, where you have the library installed, this project cannot say.

[rliebi]

We have a similar issue with our project where a library is having a security vulnerability which only can be exploited by logged in admin users. The fix is in the next Major version which is in ALPHA state, which is also suggested by the CVE. (the major version 11 which is not released yet)

So maybe there could be a check if when the fix version is not released yet, we could accept the package anyways? because what alternative do we have? a whitelist for a specific version would be fantastic.

(besides not requiring this security bundle)

https://huntr.dev/bounties/04447124-c7d4-477f-8364-91fe5b59cda0/

[Ocramius]

I don't think that's reasonably feasible: we only take authoritative sources here, and as you can see, maintainers already do enough mistakes.

Packages are sometimes not even released via tagging.

[LosD]

Damn. Was good while it lasted.This package is impossible to use in a sane way without whitelists. Removing.

For anyone that needs checks to continue even though a specific package cannot be updated right now: Use e.g. https://github.com/fabpot/local-php-security-checker instead. Not as convenient, but you can temporarily ignore packages (or filter if using in an automated script)

[Ocramius]

@LosD AFAIK, there is one way to allow-list an entry:

{
  "require": {
    "some/unsafe-package": "1.2.3 AS 99.99.99"
  }
}

I'm not 100% sure this will work, but it should trick SAT into thinking that the constraint is completely safe.

That said, security takes priority over:

• product
• bugs
• backwards compatibility

A broken or non-functioning tool is better than a compromised one.

[LosD]

@Ocramius The issue with that stance is that a lot of CVRs are issued when it is easy to misconfigure a package (see https://nvd.nist.gov/vuln/detail/CVE-2021-46743 which was the example for one of the requests from @josefsabl above), and others only covers certain uses. A lot of package maintainers will (understandably) not upgrade to sacrifice backwards compatibility for something that for their use case is a non-issue with a CVR number.

This all or nothing approach means worse security, not better.

[Ocramius]

Hard disagree on that, but then I'll gladly accept patches to generate manifests for each CVSS level, if you are willing to take that kind of risk upon you :P

This package is here as a clear exclusion list: that is documented in the first lines @ https://github.com/Roave/SecurityAdvisories/tree/885786b30f50e83a88941df2062d68de623599fd#usage

The fact that companies won't upgrade, then suffer the woes of that, is just the general misunderstanding that OSS is free as in "your problem now" :P

[Ocramius]

Meanwhile, locking here: I've made it clear that this package is just here to be used as an exclusion list that effectively shadows installable package versions.

Further feedback on this approach would need to come in form of PRs to https://github.com/Roave/SecurityAdvisoriesBuilder