Java: Add XXE sink model for Woodstox WstxInputFactory#21718
Java: Add XXE sink model for Woodstox WstxInputFactory#21718owen-mc merged 2 commits intogithub:mainfrom
Conversation
|
Thanks for this contribution. I've started CI. Just to be clear, the logic is exactly the same as |
|
Hey, thanks for looking. Yes indeed, I didn't want to mess with the core QLL |
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
Thank you for this contribution.
The way you've done it works fine, but I don't like the large amount of duplication. I think it would be better to make a small change to java/ql/lib/semmle/code/java/security/XmlParsers.qll by defining WstxInputFactory and replacing
m.getDeclaringType() instanceof XmlInputFactory and
with
(
m.getDeclaringType() instanceof XmlInputFactory or
m.getDeclaringType() instanceof WstxInputFactory
) and
Also please update the QLDoc above XmlInputFactoryStreamReader.
`com.ctc.wstx.stax.WstxInputFactory` overrides `createXMLStreamReader`, `createXMLEventReader` and `setProperty` from `XMLInputFactory`, so the existing `XmlInputFactory` model in `XmlParsers.qll` does not match calls where the static receiver type is `WstxInputFactory` (or its supertype `org.codehaus.stax2.XMLInputFactory2`). Woodstox is vulnerable to XXE in its default configuration, so these missed sinks were false negatives in `java/xxe`. This adds a scoped framework model under `semmle/code/java/frameworks/woodstox/WoodstoxXml.qll` (registered in the `Frameworks` module of `XmlParsers.qll`) that recognises these calls as XXE sinks and treats the factory as safe when both `javax.xml.stream.supportDTD` and `javax.xml.stream.isSupportingExternalEntities` are disabled — mirroring the existing `XMLInputFactory` safe-configuration logic.
chmodxxx
force-pushed
the
java/woodstox-xxe
branch
from
96d4f66 to
fb2d53e
Compare
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
Thanks again for this contribution. You did very well to create stubs and update the tests yourself.
2 participants
com.ctc.wstx.stax.WstxInputFactoryand its abstract parentorg.codehaus.stax2.XMLInputFactory2overridecreateXMLStreamReader,createXMLEventReader, andsetProperty, so calls with those static receiver types were not matched by the existingXmlInputFactorymodel. Woodstox is XXE-vulnerable out of the box, so these were real false negatives injava/xxe.