-
-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exclude packages from security review #27
Comments
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.
That is true indeed, but that's also why this package acts at
This can be done in a way that is compatible with this module by using
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 |
Well, there's this: https://security.snyk.io/vuln/SNYK-PHP-FIREBASEPHPJWT-2434829 I was looking for a way to "whitelist" a vulnerable package to find this thread. Now I need to remove security advisories altogether 😥. |
TBH, sounds like you have a security issue, and you need to help with the affected library upgrade, or maintain your own fork of You are just shooting at the messenger here. |
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. |
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. |
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/ |
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. |
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) |
@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:
A broken or non-functioning tool is better than a compromised one. |
@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. |
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 |
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 |
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.
The text was updated successfully, but these errors were encountered: