-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Limit the XML nesting depth for CVE-2022-45688 #720
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - the only questionable decision is to disable the depth limit by default, as dependent projects will still be vulnerable, if they do not change the invocation on their side.
I agree I think there should be a reasonable default set. and people need to override it if they need more depth. |
@cleydyr Sorry, my preference of not enforcing a default limit was not the best idea. Can you set a non-negative default value instead? ChatGPT suggests several hundred, which sounds reasonable to me. |
eb56704
What problem does this code solve? Risks Changes to the API? Will this require a new release? Should the documentation be updated? Does it break the unit tests? Was any code refactored in this commit? Review status |
Starting 3 day comment window |
A number of people have raised the valid point that JSONML and JsonObject/Array should have this protection as well. |
Personally I'd like a fast release to address the CVE, if the XML parser
fix resolves it.
tor. 9. feb. 2023 19.00 skrev Sean Leary ***@***.***>:
… A number of people have raised the valid point that JSONML and
JsonObject/Array should have this protection as well.
No objection if anyone wants to address this, to be included in the next
release.
We don't have an equivalent to XMLParserConfiguration, so something will
have to be done about that - either add parser config types or come up with
some other way to configure the parsers. My preference would be to add new
parser config types, but I am open to other approaches. See #722
<#722>
—
Reply to this email directly, view it on GitHub
<#720 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAF2N27KX33MSIHWEXOTM5TWWUWFVANCNFSM6AAAAAAUN34BLQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@stleary: Since #708 and #722 are both fixed, when may we expect a new release that contains these fixes for CVE-2022-4568? |
Should be later this week. |
Hi there, very quick check. A week has passed (from last comment, that release should be later this week). Do you guys have any ETA of this release? 🙏 Normally, I wouldn't push it but as one of issues have quite high CVSS score, it is kind of a deal for us :-) |
Same here, we're considering alternatives because of the critical score. |
I can release it now, so ETA = today |
Released, and should appear in the Maven repo in the next few hours. |
Its in Maven Central! Thank you everyone! |
Thanks a lot @stleary ;) |
I received this vulnerability when using jackson-core but I don't see anything updated so far. Can you provide the link to the maven update? Thanks. jackson-core-2.14.1.jar A stack overflow in the XML.toJSONObject component of hutool-json v5.8.10 allows attackers to cause a Denial of Service (DoS) via crafted JSON or XML data. I don't see any libraries called Hutool in the dependency analyzer. |
There are a few issues around this vulnerability:
Suggestions:
|
Excellent write-up, I followed all the steps and this worked great. Thank you and I appreciate it. |
dependencyCheck v 8.1.1 json-20230227.jar (pkg:maven/org.json/json@20230227, cpe:2.3:a:json-java_project:json-java::::::::) : CVE-2022-45688 no other versions of org.json are being referenced |
Like @TamasPergerDWP commented, I think the problem is that NIST DB has not been updated yet. |
@d-miller-1 please also note jeremylong/DependencyCheck#5545 |
Fixes #708