-
-
Notifications
You must be signed in to change notification settings - Fork 21
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #62 from tonybaloney/xmlstdlib
Add XML DoS inspection and tests
- Loading branch information
Showing
10 changed files
with
242 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # XML100 | ||
|
|
||
| If your application ever loads and parses XML files, the odds are you are using one of the XML standard library modules. There are a few common attacks through XML. Mostly DoS-style (designed to crash systems instead of exfiltration of data). Those attacks are common, especially if you’re parsing external (ie non-trusted) XML files. | ||
|
|
||
| One of those is called “billion laughs”, because of the payload normally containing a lot (billions) of “lols”. Basically, the idea is that you can do referential entities in XML, so when your unassuming XML parser tries to load this XML file into memory it consumes gigabytes of RAM. Try it out if you don’t believe me :-) | ||
|
|
||
| ```xml | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE lolz [ | ||
| <!ENTITY lol "lol"> | ||
| <!ENTITY lol2 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;"> | ||
| <!ENTITY lol3 "&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;"> | ||
| <!ENTITY lol4 "&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;"> | ||
| <!ENTITY lol5 "&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;"> | ||
| <!ENTITY lol6 "&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;"> | ||
| <!ENTITY lol7 "&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;"> | ||
| <!ENTITY lol8 "&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;"> | ||
| <!ENTITY lol9 "&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;"> | ||
| ]> | ||
| <lolz>&lol9;</lolz> | ||
| ``` | ||
|
|
||
| Another attack uses external entity expansion. XML supports referencing entities from external URLs, the XML parser would typically fetch and load that resource without any qualms. “An attacker can circumvent firewalls and gain access to restricted resources as all the requests are made from an internal and trustworthy IP address, not from the outside.” | ||
|
|
||
| Another situation to consider is 3rd party packages you’re depending on that decode XML, like configuration files, remote APIs. You might not even be aware that one of your dependencies leaves itself open to these types of attacks. | ||
|
|
||
| So what happens in Python? Well, the standard library modules, etree, DOM, xmlrpc are all wide open to these types of attacks. | ||
|
|
||
| ## Fix | ||
|
|
||
| * Use [defusedxml](https://pypi.org/project/defusedxml/) as a drop-in replacement for the standard library modules. It adds safe-guards against these types of attacks. | ||
|
|
||
| ## See Also | ||
|
|
||
| * [Official Documentation on XMl vulnerabilities](https://docs.python.org/3/library/xml.html#xml-vulnerabilities) | ||
| * [defusedxml package on PyPi](https://pypi.org/project/defusedxml/) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
src/main/java/security/validators/StandardLibraryXmlInspection.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package security.validators | ||
|
|
||
| import com.intellij.codeInspection.LocalInspectionToolSession | ||
| import com.intellij.codeInspection.ProblemsHolder | ||
| import com.intellij.psi.PsiElementVisitor | ||
| import com.jetbrains.python.inspections.PyInspection | ||
| import com.jetbrains.python.inspections.PyInspectionVisitor | ||
| import com.jetbrains.python.psi.PyFromImportStatement | ||
| import com.jetbrains.python.psi.PyImportStatement | ||
| import security.Checks | ||
|
|
||
| class StandardLibraryXmlInspection : PyInspection() { | ||
| val check = Checks.StandardLibraryXmlCheck | ||
|
|
||
| override fun getStaticDescription(): String? { | ||
| return check.getStaticDescription() | ||
| } | ||
|
|
||
| override fun buildVisitor(holder: ProblemsHolder, | ||
| isOnTheFly: Boolean, | ||
| session: LocalInspectionToolSession): PsiElementVisitor = Visitor(holder, session) | ||
|
|
||
| private class Visitor(holder: ProblemsHolder, session: LocalInspectionToolSession) : PyInspectionVisitor(holder, session) { | ||
| val vulnerableNamespaces = arrayOf("xml.sax", "xml.etree", "xml.dom.minidom", "xml.dom.pulldom", "xmlrpc.server") | ||
|
|
||
| private fun match(qname: String): Boolean { | ||
| return (listOf(*vulnerableNamespaces).any{ qname.startsWith(it) }) | ||
| } | ||
|
|
||
| override fun visitPyFromImportStatement(node: PyFromImportStatement) { | ||
| if (node.importSourceQName == null) return | ||
| if (node.importSourceQName!!.toString().isEmpty()) return | ||
| if (match(node.importSourceQName!!.toString())) | ||
| holder?.registerProblem(node, Checks.StandardLibraryXmlCheck.getDescription()) | ||
| } | ||
|
|
||
| override fun visitPyImportStatement(node: PyImportStatement) { | ||
| if (node.fullyQualifiedObjectNames.isEmpty()) return | ||
| node.fullyQualifiedObjectNames | ||
| .filter { match(it) } | ||
| .forEach { holder?.registerProblem(node, Checks.StandardLibraryXmlCheck.getDescription()) } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,17 @@ | ||
| <idea-plugin> | ||
| <id>org.tonybaloney.security.pycharm-security</id> | ||
| <name>Python Security</name> | ||
| <vendor email="[email protected]" url="http://tonybaloney.github.io">Anthony Shaw</vendor> | ||
| <vendor email="[email protected]" url="https://github.com/tonybaloney/pycharm-security/">Anthony Shaw</vendor> | ||
|
|
||
| <description><![CDATA[ | ||
| Analyzes potential security flaws in your Python code and suggests fixes. Designed to find common security issues in Python code.<br> | ||
| Reports on installed packages and any known security reports. <br/> | ||
| Also available for your CI/CD as a GitHub Action <a href="https://github.com/marketplace/actions/pycharm-python-security-scanner">on the GitHub marketplace</a>. | ||
| <h2>Features</h2> | ||
| <ul> | ||
| <li>Over 20 builtin code checks giving your contextual security warnings in your code</li> | ||
| <li>Over 40 builtin code checks giving your contextual security warnings in your code</li> | ||
| <li>Misconfiguration warnings for Django and Flask web frameworks</li> | ||
| <li>Cross-Site-Scripting detection for both Jinja2 and Mako templating engines</li> | ||
| <li>SQL Injection detection in all Python string formats</li> | ||
|
|
@@ -53,6 +55,7 @@ | |
| <localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" level="WEAK WARNING" displayName="NET100: Possible service binding to all network interfaces." shortName="BindAllInterfacesInspection" implementationClass="security.validators.BindAllInterfacesInspection" /> | ||
| <localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="OS100: Call to os.chmod setting permission values." shortName="OsChmodInspection" implementationClass="security.validators.OsChmodInspection" /> | ||
| <localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="PIC100: Loading serialized data with the pickle module." shortName="PickleLoadInspection" implementationClass="security.validators.PickleLoadInspection" /> | ||
| <localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="XML100: Usage of xml modules in the Python standard library." shortName="StandardLibraryXmlInspector" implementationClass="security.validators.StandardLibraryXmlInspection" /> | ||
| <localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="XML200: Using SimpleXMLRPCServer with 'allow_dotted_names'." shortName="XmlRpcServerDottedNamesInspection" implementationClass="security.validators.XmlRpcServerDottedNamesInspection" /> | ||
| </extensions> | ||
| <extensions defaultExtensionNs="Pythonid"> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
src/test/java/security/validators/StandardLibraryXmlInspectionTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| package security.validators | ||
|
|
||
| import org.junit.jupiter.api.AfterAll | ||
| import org.junit.jupiter.api.BeforeAll | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.TestInstance | ||
| import security.Checks | ||
| import security.SecurityTestTask | ||
|
|
||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| class StandardLibraryXmlInspectionTest: SecurityTestTask() { | ||
| @BeforeAll | ||
| override fun setUp() { | ||
| super.setUp() | ||
| } | ||
|
|
||
| @AfterAll | ||
| override fun tearDown(){ | ||
| super.tearDown() | ||
| } | ||
|
|
||
| @Test | ||
| fun `verify description is not empty`(){ | ||
| assertFalse(StandardLibraryXmlInspection().staticDescription.isNullOrEmpty()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test invalid import`(){ | ||
| var code = """ | ||
| from import x | ||
| """.trimIndent() | ||
| testFromImportStatement(code, 0, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test from import`(){ | ||
| var code = """ | ||
| from xml.dom.minidom import parse, parseString | ||
| """.trimIndent() | ||
| testFromImportStatement(code, 1, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test normal import minidom`(){ | ||
| var code = """ | ||
| import xml.dom.minidom | ||
| """.trimIndent() | ||
| testImportStatement(code, 1, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test normal safe namespace`(){ | ||
| var code = """ | ||
| import defusedxml.minidom | ||
| """.trimIndent() | ||
| testImportStatement(code, 0, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test alias import`(){ | ||
| var code = """ | ||
| import xml.etree.ElementTree as ET | ||
| """.trimIndent() | ||
| testImportStatement(code, 1, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test multiple import`(){ | ||
| var code = """ | ||
| import xml.etree.ElementTree, xml.dom.minidom | ||
| """.trimIndent() | ||
| testImportStatement(code, 2, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test nested import`(){ | ||
| var code = """ | ||
| def foo(): | ||
| import xml.etree.ElementTree as ET | ||
| """.trimIndent() | ||
| testImportStatement(code, 1, Checks.StandardLibraryXmlCheck, "test_foo.py", StandardLibraryXmlInspection()) | ||
| } | ||
| } |