Skip to content

[SLING-12492] Upgrade the dependencies affected by CVE-2022-32549 an… #3

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

Closed
wants to merge 0 commits into from

Conversation

yuansc
Copy link

@yuansc yuansc commented Nov 15, 2024

…d CVE-2021-29425 to their latest available versions to address these vulnerabilities.

@rombert
Copy link
Contributor

rombert commented Nov 20, 2024

Thanks for the PR @yuansc . I understand why vulnerability scanners flag this bundle (and many other Sling ones ) as vulnerable but in fact there is no vulnerability.

These dependencies are marked as provided so they are used when building locally, running tests, and when generating import range for OSGi bundles. At deployment time a different version of those bundles will be used, and that is what the deployer defines.

For instance, in the Sling Starter we currently use

As you can see, the vulnerable versions are not used at runtime.

If we were to merge this PR then we would modify the import ranges of this bundle to no longer work with older verisons of commons-io and org.apache.sling.api. While I personally think it's a bad idea to deploy vulnerable versions, we can't force our users to do that, it must be their action.

For this reason we don't usually update dependencies - see https://cwiki.apache.org/confluence/display/SLING/Dependabot for more details, even though it's about automatic updates.

@enapps-enorman
Copy link
Contributor

enapps-enorman commented Nov 20, 2024

If we were to merge this PR then we would modify the import ranges of this bundle to no longer work with older verisons of commons-io and org.apache.sling.api. While I personally think it's a bad idea to deploy vulnerable versions, we can't force our users to do that, it must be their action.

For this reason we don't usually update dependencies

@rombert - Maybe we could have another vote on that? I'd still prefer these dependencies to be the oldest compatible version that doesn't have known security vulnerabilities just to keep these scanners happy.

I get a weekly "security alert digest" email from github for all the projects with "known security vulnerabilities detected". That email has become almost useless because it only contains the first 10 hits and all of them are apache/sling projects impacted by this type of problem. So I may never see "real" security problems due to all the noise that we could clean up by changing our approach to such things.

For example, for this project the dependencies could be the following versions that are not reported as having vulnerabilities:

  • org.apache.sling.api version 2.25.4 which was released over 2 years ago
  • commons-io version 2.14.0 which was released over 1 year ago

I would think a year is more than enough time for everyone to upgrade to non-vulnerable versions of those?

@yuansc
Copy link
Author

yuansc commented Nov 21, 2024

Hello @rombert,

Thank you for providing the reference and detailed explanations regarding the impacts of the dependencies update. Your insights are very helpful, and I completely agree that considering the broader user base and selecting libraries during packaging can better support those utilizing this project's work.

The misleading vulnerabilities reported by the Maven repository (e.g., https://mvnrepository.com/artifact/org.apache.sling/org.apache.sling.scripting.javascript/3.1.4) are indeed problematic, which was a key motivator for this PR and aligns somewhat with @enapps-enorman's comments.

With full respect for the process, I am okay with either dropping this PR or waiting for a vote—both options work for me.

Thank you again for your guidance.

Best regards,
Scott Yuan

@rombert
Copy link
Contributor

rombert commented Nov 21, 2024

@rombert - Maybe we could have another vote on that? I'd still prefer these dependencies to be the oldest compatible version that doesn't have known security vulnerabilities just to keep these scanners happy.

Sure @enapps-enorman , feel free to start a discussion on the dev list.

@rombert
Copy link
Contributor

rombert commented Nov 21, 2024

With full respect for the process, I am okay with either dropping this PR or waiting for a vote—both options work for me.

Let's wait for a bit and have the discussion on the dev list. Feel free to subscribe to dev@sling ( https://sling.apache.org/project-information.html#mailing-lists-1 ) and add your opinion on the topic.

@enapps-enorman
Copy link
Contributor

FYI: I proposed a vote on the dev list. The thread is archived at: https://www.mail-archive.com/dev@sling.apache.org/msg135328.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants