Skip to content

Commit

Permalink
Configure SAXParserFactory to avoid XXE references
Browse files Browse the repository at this point in the history
 #KT-51519 Fixed
  • Loading branch information
udalov committed Apr 2, 2022
1 parent d01e6b6 commit 9c78d57
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
Expand Up @@ -85,7 +85,12 @@ private void setCurrentState(@NotNull DefaultHandler currentState) {
private ModuleChunk parse(@NotNull InputStream xml) {
try {
setCurrentState(initial);
SAXParser saxParser = SAXParserFactory.newInstance().newSAXParser();
SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
SAXParser saxParser = factory.newSAXParser();
saxParser.parse(xml, new DelegatedSaxHandler() {
@NotNull
@Override
Expand Down
Expand Up @@ -30,7 +30,6 @@ import org.xml.sax.SAXException
import org.xml.sax.helpers.DefaultHandler
import java.io.IOException
import java.io.Reader
import java.util.*
import javax.xml.parsers.SAXParserFactory

object CompilerOutputParser {
Expand Down Expand Up @@ -59,6 +58,10 @@ object CompilerOutputParser {
}
try {
val factory = SAXParserFactory.newInstance()
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true)
factory.setFeature("http://xml.org/sax/features/external-general-entities", false)
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false)
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false)
val parser = factory.newSAXParser()
parser.parse(InputSource(wrappingReader), CompilerOutputSAXHandler(messageCollector, collector))
}
Expand Down

8 comments on commit 9c78d57

@JamieSlome
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@udalov - I believe that we initially received this report here:
https://www.huntr.dev/bounties/e9b2b1dd-ab65-48e2-8042-b53253649961

Can you confirm that this has been addressed via this commit SHA?

@udalov
Copy link
Member Author

@udalov udalov commented on 9c78d57 Apr 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamieSlome I don't have an account at that website so I can't confirm. But if it's the same problem as reported in https://youtrack.jetbrains.com/issue/KT-51519, then yes.

@ready-research
Copy link

@ready-research ready-research commented on 9c78d57 Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@udalov I gave access to you for KT-51519, can you please confirm that this fixing the issue reported in huntr using the comments in the report KT-51519
"I originally reported this in huntr https://www.huntr.dev/bounties/e9b2b1dd-ab65-48e2-8042-b53253649961/"

@JamieSlome
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@udalov - I cannot get access to KT-51519, would you be able to e-mail me at jamie@418sec.com with the details of the issue, or perhaps I can share the details of the report we have received, and we can confirm whether the two reports are the same?

@udalov
Copy link
Member Author

@udalov udalov commented on 9c78d57 Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamieSlome I guess it is the same issue, because the huntr link you've provided matches with the link originally reported by @ready-research in KT-51519 (which is now indeed private).

@JamieSlome
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@udalov - we would just be eager to compare if possible, just so we can check the report has been validated/approved by your team.

Could I send you the details via e-mail received in our report, just in case our reports don't match up? Don't want to share a potential vulnerability that may not have been addressed by your team. Let me know what works for you 👍

@udalov
Copy link
Member Author

@udalov udalov commented on 9c78d57 Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've sent you an email.

@JamieSlome
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate it @udalov 👍

Please sign in to comment.