-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Unlimited recursion for DTD parsing #157
Comments
WTF. Any info on what those are? No one has contacted me. |
This notification is from our projects whitesource check. As I use it in a controlled environment, my concern is mainly about whitesource freaking out and security folks asking when the issue will be resolved. |
@StefanSchrass At this point I don't have time to go looking for issues, so someone should explain exactly what is supposed to be the problem; maybe go via Tidelift security contact OR email me to my fasterxml email address ( A smaller think that'd help would be linking to CVE description, although maybe actual description are not yet open to public. I really think it's a particularly shitty thing to file for an CVE before contacting library author; not sure if that's what happened here but certainly smells like that. If so, shame on people filing these. |
Huh? https://nvd.nist.gov/vuln/detail/CVE-2022-40151 is for XStream NOT Woodstox. It also seems to be from questionable source: so... I don't think I should spend any time yet until something tangible is reported. |
I think the problem is centered around woodstox-core is using Xstream. |
This is nuts. And I do hope that database gets updated. |
Yeah, I am really unhappy about some of security research activity around OSS. Also forgot: thank you @StefanSchrass for bringing this to my attention. I did not mean to direct my frustration towards you, just in case that was not clear. |
hi @cowtowncoder, do you want to be added temporarily as receiver for the xstream bugs in oss-fuzz. This would grant you with access to the stacktraces and crashing inputs. I could point you to the few issues that might be related to woodstox. Sorry for the inconvenience. |
@0roman Thank you for the offer but I don't think I have to time to pro-actively dig into issues here, due to overload with many other things (I get reports for all Jackson-related fuzzers and one Woodstox one), as well as not being familiar with XStream codebase these days. To work on Woodstox issues the problem would need to be isolated anyway, and I think security researchers would be in a good position to do that; once more isolated examples exist I would definitely help in figuring out what is going on. I appreciate your reaching out, I hope we can figure out the issues. |
@cowtowncoder okay, I completely understand that. I am also not really familiar with XStream yet, however I have spend some time going quickly over those issues in OSS-Fuzz. I have observed that readContenSpec in woodstox class FullDTDReader
malformed_XML.zip. That behaviour might be fixed by some maximum recursion depth parameter. I hope that helps a little bit. If more investigation is necessary, I will be happy to assist. |
On Wed, Sep 28, 2022 at 8:24 AM Roman Wagner ***@***.***> wrote:
@cowtowncoder <https://github.com/cowtowncoder> okay, I completely
understand that.
I am also not really familiar with XStream yet, however I have spend some
time going quickly over those issues in OSS-Fuzz. I have observed that
readContenSpec in woodstox class FullDTDReader
https://github.com/FasterXML/woodstox/blob/b8f136b299de39064c957a76261e104787e89aad/src/main/java/com/ctc/wstx/dtd/FullDTDReader.java#L3050
is called recursively some hundred times before leading to the
stackoverflows in all issues. From the code it looks like that every new
bracket "(" might call readContentSpec recursively. The crashing inputs are
also indicating that XML documents that are containing some hundred open
brackets but no closed one may reproduce that issue, such as
malformed_XML.zip
<https://github.com/FasterXML/woodstox/files/9666374/clusterfuzz-testcase-minimized-XmlFuzzer-5219006592450560.zip>
.
That behaviour might be fixed by some maximum recursion depth parameter. I
hope that helps a little bit. If more investigation is necessary, I will be
happy to assist.
Ok, so this would be about parsing of DTDs (external and internal subsets),
not actual XML content.
That makes sense since limits already exist for XML content, but probably
not yet for DTD handling
(there are other checks for circular definitions etc but not for
non-circular nesting).
An issue filed for Woodstox would make sense, with above info.
Severity of this is probably somewhat low just because everyone who is
security focused almost certainly disables DTD handling
anyway: but it is still a good thing to fix.
-+ Tatu +-
… Message ID: ***@***.***>
|
@cowtowncoder great to hear that, is a public issue fine for you? I agree, although it might be relevant for the severity what the default settings are. According to the fuzz target (https://github.com/google/oss-fuzz/blob/master/projects/xstream/XmlFuzzer.java) the vulnerability in xstream was just triggered with some default xstream settings. |
@0roman yes, public issue is fine. |
Aside from patch, can anybody figure out which CVE(s) this specifically addresses? All 6? One of them? @pjfanning thank you; I hope to check this ASAP. |
@cowtowncoder all 6 have exactly the same unhelpful description - I think we should appeal and get all but one closed and use the one remaining one used for #159 change |
Yep, we tried, didn't happen: x-stream/xstream#304 (comment). |
Alas! It's a shame that there's probably some incentives to file multiple CVEs, take credit, even if the root causes are the same. I can also understand how it may be difficult to tell from symptoms / reproductions how many distinct issues there are. Still, in this case vagueness should be considered in consolidating them. But it is what it is I guess. |
Whops. I filed #160 as I forgot there was this one already, so release notes refer to that. |
I just got notified that:
The text was updated successfully, but these errors were encountered: