From 88520435584f3ed3300e69f3d388a5dc0d07d11d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 5 May 2022 17:02:06 +0100 Subject: [PATCH 1/6] C++: Additional test cases. --- .../Security/CWE/CWE-611/tests5.cpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp index e98d5a99e604..b55b7bab1e90 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp @@ -76,3 +76,28 @@ void test5_6() { g_p1->parse(*g_data); // GOOD g_p2->parse(*g_data); // BAD (parser not correctly configured) [NOT DETECTED] } + +void test5_7(DOMImplementationLS *impl, InputSource &data) { + DOMLSParser *p = impl->createLSParser(); + + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + + p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); + p->parse(data); // GOOD + + p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] +} + +void test5_8(DOMImplementationLS *impl, InputSource &data) { + DOMLSParser *p = impl->createLSParser(); + DOMConfiguration *cfg = p->getDomConfig(); + + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + + cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); + p->parse(data); // GOOD + + cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] +} From e3be7749ea1baa80732e6e0ea94c52acdab377c0 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 5 May 2022 16:54:27 +0100 Subject: [PATCH 2/6] C++: Repair the LSParser sinks. --- cpp/ql/src/Security/CWE/CWE-611/XXE.ql | 13 ++++-- .../Security/CWE/CWE-611/XXE.expected | 40 +++++++++++++++++++ .../Security/CWE/CWE-611/tests5.cpp | 22 +++++----- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql index c8b638ddecdc..d56e6c56bc6c 100644 --- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql @@ -57,6 +57,13 @@ class XercesDOMParserClass extends Class { XercesDOMParserClass() { this.hasName("XercesDOMParser") } } +/** + * The `DOMLSParser` class. + */ +class DOMLSParserClass extends Class { + DOMLSParserClass() { this.hasName("DOMLSParser") } +} + /** * The `SAXParser` class. */ @@ -217,12 +224,12 @@ class SetFeatureTranformer extends XXEFlowStateTranformer { } /** - * The `AbstractDOMParser.parse`, `SAXParser.parse` or `SAX2XMLReader.parse` - * method. + * The `AbstractDOMParser.parse`, `DOMLSParserClass.parse`, `SAXParser.parse` or `SAX2XMLReader.parse` method. */ class ParseFunction extends Function { ParseFunction() { this.getClassAndName("parse") instanceof AbstractDOMParserClass or + this.getClassAndName("parse") instanceof DOMLSParserClass or this.getClassAndName("parse") instanceof SaxParserClass or this.getClassAndName("parse") instanceof Sax2XmlReader } @@ -235,7 +242,7 @@ class ParseFunction extends Function { class CreateLSParser extends Function { CreateLSParser() { this.hasName("createLSParser") and - this.getUnspecifiedType().(PointerType).getBaseType().getName() = "DOMLSParser" // returns a `DOMLSParser *`. + this.getUnspecifiedType().(PointerType).getBaseType() instanceof DOMLSParserClass // returns a `DOMLSParser *`. } } diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected index 24371939674d..3ffb719a96b7 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected @@ -4,6 +4,17 @@ edges | tests3.cpp:23:21:23:53 | call to createXMLReader | tests3.cpp:25:2:25:2 | p | | tests3.cpp:60:21:60:53 | call to createXMLReader | tests3.cpp:63:2:63:2 | p | | tests3.cpp:67:21:67:53 | call to createXMLReader | tests3.cpp:70:2:70:2 | p | +| tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p | +| tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p | +| tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p | +| tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p | +| tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p | +| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | +| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86:2 | p | +| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p | +| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p | +| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p | +| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102:2 | p | | tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p | | tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p | | tests.cpp:35:19:35:19 | VariableAddress [post update] | tests.cpp:37:2:37:2 | p | @@ -46,6 +57,24 @@ nodes | tests4.cpp:46:34:46:68 | ... \| ... | semmle.label | ... \| ... | | tests4.cpp:77:34:77:38 | flags | semmle.label | flags | | tests4.cpp:130:39:130:55 | (int)... | semmle.label | (int)... | +| tests5.cpp:27:25:27:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:29:2:29:2 | p | semmle.label | p | +| tests5.cpp:33:25:33:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:36:2:36:2 | p | semmle.label | p | +| tests5.cpp:40:25:40:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:43:2:43:2 | p | semmle.label | p | +| tests5.cpp:47:25:47:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:51:2:51:2 | p | semmle.label | p | +| tests5.cpp:55:25:55:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:59:2:59:2 | p | semmle.label | p | +| tests5.cpp:81:25:81:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:83:2:83:2 | p | semmle.label | p | +| tests5.cpp:86:2:86:2 | p | semmle.label | p | +| tests5.cpp:89:2:89:2 | p | semmle.label | p | +| tests5.cpp:93:25:93:38 | call to createLSParser | semmle.label | call to createLSParser | +| tests5.cpp:96:2:96:2 | p | semmle.label | p | +| tests5.cpp:99:2:99:2 | p | semmle.label | p | +| tests5.cpp:102:2:102:2 | p | semmle.label | p | | tests.cpp:15:23:15:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument | | tests.cpp:17:2:17:2 | p | semmle.label | p | | tests.cpp:28:23:28:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument | @@ -93,6 +122,17 @@ subpaths | tests4.cpp:46:34:46:68 | ... \| ... | tests4.cpp:46:34:46:68 | ... \| ... | tests4.cpp:46:34:46:68 | ... \| ... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:46:34:46:68 | ... \| ... | XML parser | | tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:77:34:77:38 | flags | XML parser | | tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:130:39:130:55 | (int)... | XML parser | +| tests5.cpp:29:2:29:2 | p | tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:27:25:27:38 | call to createLSParser | XML parser | +| tests5.cpp:36:2:36:2 | p | tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:33:25:33:38 | call to createLSParser | XML parser | +| tests5.cpp:43:2:43:2 | p | tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:40:25:40:38 | call to createLSParser | XML parser | +| tests5.cpp:51:2:51:2 | p | tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:47:25:47:38 | call to createLSParser | XML parser | +| tests5.cpp:59:2:59:2 | p | tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:55:25:55:38 | call to createLSParser | XML parser | +| tests5.cpp:83:2:83:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | +| tests5.cpp:86:2:86:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | +| tests5.cpp:89:2:89:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | +| tests5.cpp:96:2:96:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | +| tests5.cpp:99:2:99:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | +| tests5.cpp:102:2:102:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | | tests.cpp:17:2:17:2 | p | tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:15:23:15:43 | XercesDOMParser output argument | XML parser | | tests.cpp:31:2:31:2 | p | tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:28:23:28:43 | XercesDOMParser output argument | XML parser | | tests.cpp:39:2:39:2 | p | tests.cpp:35:23:35:43 | XercesDOMParser output argument | tests.cpp:39:2:39:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:35:23:35:43 | XercesDOMParser output argument | XML parser | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp index b55b7bab1e90..2716a708c320 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp @@ -26,21 +26,21 @@ class DOMImplementationLS { void test5_1(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) } void test5_2(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD + p->parse(data); // GOOD [FALSE POSITIVE] } void test5_3(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) } void test5_4(DOMImplementationLS *impl, InputSource &data) { @@ -48,7 +48,7 @@ void test5_4(DOMImplementationLS *impl, InputSource &data) { DOMConfiguration *cfg = p->getDomConfig(); cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD + p->parse(data); // GOOD [FALSE POSITIVE] } void test5_5(DOMImplementationLS *impl, InputSource &data) { @@ -56,7 +56,7 @@ void test5_5(DOMImplementationLS *impl, InputSource &data) { DOMConfiguration *cfg = p->getDomConfig(); cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) } DOMImplementationLS *g_impl; @@ -80,24 +80,24 @@ void test5_6() { void test5_7(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD + p->parse(data); // GOOD [FALSE POSITIVE] p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) } void test5_8(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); DOMConfiguration *cfg = p->getDomConfig(); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD + p->parse(data); // GOOD [FALSE POSITIVE] cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); - p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] + p->parse(data); // BAD (parser not correctly configured) } From 3dddc560a1a1470f9a7579a433a00eb988fe6f19 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 5 May 2022 18:16:50 +0100 Subject: [PATCH 3/6] C++: Add LSParser specific transformer. --- cpp/ql/src/Security/CWE/CWE-611/XXE.ql | 62 +++++++++++++++++++ .../Security/CWE/CWE-611/XXE.expected | 29 +++------ .../Security/CWE/CWE-611/tests5.cpp | 12 ++-- 3 files changed, 76 insertions(+), 27 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql index d56e6c56bc6c..ab6ef6229de5 100644 --- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql @@ -223,6 +223,68 @@ class SetFeatureTranformer extends XXEFlowStateTranformer { } } +/** + * The `DOMLSParser.getDomConfig` function. + */ +class GetDomConfig extends Function { + GetDomConfig() { + this.hasName("getDomConfig") and + this.getDeclaringType() instanceof DOMLSParserClass + } +} + +/** + * The `DOMConfiguration.setParameter` function. + */ +class DomConfigurationSetParameter extends Function { + DomConfigurationSetParameter() { + this.hasName("setParameter") and + this.getDeclaringType().getName() = "DOMConfiguration" + } +} + +/** + * A flow state transformer for a call to `DOMConfiguration.setParameter` + * specifying the feature `XMLUni::fgXercesDisableDefaultEntityResolution`. + * This is a slightly more complex transformer because the qualifier is a + * `DOMConfiguration` pointer returned by `DOMLSParser.getDomConfig` - and it + * is *that* qualifier we want to transform the flow state of. + */ +class DOMConfigurationSetParameterTranformer extends XXEFlowStateTranformer { + Expr newValue; + + DOMConfigurationSetParameterTranformer() { + exists(FunctionCall getDomConfigCall, FunctionCall setParameterCall | + // this is the qualifier of a call to `DOMLSParser.getDomConfig`. + getDomConfigCall.getTarget() instanceof GetDomConfig and + this = getDomConfigCall.getQualifier() and + // `setParameterCall` is a call to `setParameter` on the return value of + // the same call to `DOMLSParser.getDomConfig`. + setParameterCall.getTarget() instanceof DomConfigurationSetParameter and + globalValueNumber(setParameterCall.getQualifier()).getAnExpr() = getDomConfigCall and + // the parameter being set is + // `XMLUni::fgXercesDisableDefaultEntityResolution`. + globalValueNumber(setParameterCall.getArgument(0)).getAnExpr().(VariableAccess).getTarget() + instanceof FeatureDisableDefaultEntityResolution and + // the value being set is `newValue`. + newValue = setParameterCall.getArgument(1) + ) + } + + final override XXEFlowState transform(XXEFlowState flowstate) { + exists(int createEntityReferenceNodes | + encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and + ( + globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // true + encodeXercesFlowState(result, 1, createEntityReferenceNodes) + or + not globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // false or unknown + encodeXercesFlowState(result, 0, createEntityReferenceNodes) + ) + ) + } +} + /** * The `AbstractDOMParser.parse`, `DOMLSParserClass.parse`, `SAXParser.parse` or `SAX2XMLReader.parse` method. */ diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected index 3ffb719a96b7..a4db6155e31e 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected @@ -5,16 +5,14 @@ edges | tests3.cpp:60:21:60:53 | call to createXMLReader | tests3.cpp:63:2:63:2 | p | | tests3.cpp:67:21:67:53 | call to createXMLReader | tests3.cpp:70:2:70:2 | p | | tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p | -| tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p | | tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p | -| tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p | | tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p | | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | -| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86:2 | p | -| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p | -| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p | -| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p | -| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102:2 | p | +| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | +| tests5.cpp:83:2:83:2 | p | tests5.cpp:85:2:85:2 | p | +| tests5.cpp:85:2:85:2 | p | tests5.cpp:86:2:86:2 | p | +| tests5.cpp:86:2:86:2 | p | tests5.cpp:88:2:88:2 | p | +| tests5.cpp:88:2:88:2 | p | tests5.cpp:89:2:89:2 | p | | tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p | | tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p | | tests.cpp:35:19:35:19 | VariableAddress [post update] | tests.cpp:37:2:37:2 | p | @@ -59,22 +57,17 @@ nodes | tests4.cpp:130:39:130:55 | (int)... | semmle.label | (int)... | | tests5.cpp:27:25:27:38 | call to createLSParser | semmle.label | call to createLSParser | | tests5.cpp:29:2:29:2 | p | semmle.label | p | -| tests5.cpp:33:25:33:38 | call to createLSParser | semmle.label | call to createLSParser | -| tests5.cpp:36:2:36:2 | p | semmle.label | p | | tests5.cpp:40:25:40:38 | call to createLSParser | semmle.label | call to createLSParser | | tests5.cpp:43:2:43:2 | p | semmle.label | p | -| tests5.cpp:47:25:47:38 | call to createLSParser | semmle.label | call to createLSParser | -| tests5.cpp:51:2:51:2 | p | semmle.label | p | | tests5.cpp:55:25:55:38 | call to createLSParser | semmle.label | call to createLSParser | | tests5.cpp:59:2:59:2 | p | semmle.label | p | | tests5.cpp:81:25:81:38 | call to createLSParser | semmle.label | call to createLSParser | | tests5.cpp:83:2:83:2 | p | semmle.label | p | +| tests5.cpp:83:2:83:2 | p | semmle.label | p | +| tests5.cpp:85:2:85:2 | p | semmle.label | p | | tests5.cpp:86:2:86:2 | p | semmle.label | p | +| tests5.cpp:88:2:88:2 | p | semmle.label | p | | tests5.cpp:89:2:89:2 | p | semmle.label | p | -| tests5.cpp:93:25:93:38 | call to createLSParser | semmle.label | call to createLSParser | -| tests5.cpp:96:2:96:2 | p | semmle.label | p | -| tests5.cpp:99:2:99:2 | p | semmle.label | p | -| tests5.cpp:102:2:102:2 | p | semmle.label | p | | tests.cpp:15:23:15:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument | | tests.cpp:17:2:17:2 | p | semmle.label | p | | tests.cpp:28:23:28:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument | @@ -123,16 +116,10 @@ subpaths | tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:77:34:77:38 | flags | XML parser | | tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:130:39:130:55 | (int)... | XML parser | | tests5.cpp:29:2:29:2 | p | tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:27:25:27:38 | call to createLSParser | XML parser | -| tests5.cpp:36:2:36:2 | p | tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:33:25:33:38 | call to createLSParser | XML parser | | tests5.cpp:43:2:43:2 | p | tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:40:25:40:38 | call to createLSParser | XML parser | -| tests5.cpp:51:2:51:2 | p | tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:47:25:47:38 | call to createLSParser | XML parser | | tests5.cpp:59:2:59:2 | p | tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:55:25:55:38 | call to createLSParser | XML parser | | tests5.cpp:83:2:83:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | -| tests5.cpp:86:2:86:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | | tests5.cpp:89:2:89:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser | -| tests5.cpp:96:2:96:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | -| tests5.cpp:99:2:99:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | -| tests5.cpp:102:2:102:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser | | tests.cpp:17:2:17:2 | p | tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:15:23:15:43 | XercesDOMParser output argument | XML parser | | tests.cpp:31:2:31:2 | p | tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:28:23:28:43 | XercesDOMParser output argument | XML parser | | tests.cpp:39:2:39:2 | p | tests.cpp:35:23:35:43 | XercesDOMParser output argument | tests.cpp:39:2:39:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:35:23:35:43 | XercesDOMParser output argument | XML parser | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp index 2716a708c320..99027c9bd933 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-611/tests5.cpp @@ -33,7 +33,7 @@ void test5_2(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD [FALSE POSITIVE] + p->parse(data); // GOOD } void test5_3(DOMImplementationLS *impl, InputSource &data) { @@ -48,7 +48,7 @@ void test5_4(DOMImplementationLS *impl, InputSource &data) { DOMConfiguration *cfg = p->getDomConfig(); cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD [FALSE POSITIVE] + p->parse(data); // GOOD } void test5_5(DOMImplementationLS *impl, InputSource &data) { @@ -83,7 +83,7 @@ void test5_7(DOMImplementationLS *impl, InputSource &data) { p->parse(data); // BAD (parser not correctly configured) p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD [FALSE POSITIVE] + p->parse(data); // GOOD p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); p->parse(data); // BAD (parser not correctly configured) @@ -93,11 +93,11 @@ void test5_8(DOMImplementationLS *impl, InputSource &data) { DOMLSParser *p = impl->createLSParser(); DOMConfiguration *cfg = p->getDomConfig(); - p->parse(data); // BAD (parser not correctly configured) + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true); - p->parse(data); // GOOD [FALSE POSITIVE] + p->parse(data); // GOOD cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false); - p->parse(data); // BAD (parser not correctly configured) + p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED] } From 00f7453fcb86d8d0b9fd3e8ea7e2e70165d336a4 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 11 May 2022 11:08:03 +0100 Subject: [PATCH 4/6] C++: Fix capitalization. --- cpp/ql/src/Security/CWE/CWE-611/XXE.ql | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql index ab6ef6229de5..5f3ae65d78cc 100644 --- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql @@ -60,8 +60,8 @@ class XercesDOMParserClass extends Class { /** * The `DOMLSParser` class. */ -class DOMLSParserClass extends Class { - DOMLSParserClass() { this.hasName("DOMLSParser") } +class DomLSParserClass extends Class { + DomLSParserClass() { this.hasName("DOMLSParser") } } /** @@ -229,7 +229,7 @@ class SetFeatureTranformer extends XXEFlowStateTranformer { class GetDomConfig extends Function { GetDomConfig() { this.hasName("getDomConfig") and - this.getDeclaringType() instanceof DOMLSParserClass + this.getDeclaringType() instanceof DomLSParserClass } } @@ -286,12 +286,13 @@ class DOMConfigurationSetParameterTranformer extends XXEFlowStateTranformer { } /** - * The `AbstractDOMParser.parse`, `DOMLSParserClass.parse`, `SAXParser.parse` or `SAX2XMLReader.parse` method. + * The `AbstractDOMParser.parse`, `DOMLSParserClass.parse`, `SAXParser.parse` + * or `SAX2XMLReader.parse` method. */ class ParseFunction extends Function { ParseFunction() { this.getClassAndName("parse") instanceof AbstractDOMParserClass or - this.getClassAndName("parse") instanceof DOMLSParserClass or + this.getClassAndName("parse") instanceof DomLSParserClass or this.getClassAndName("parse") instanceof SaxParserClass or this.getClassAndName("parse") instanceof Sax2XmlReader } @@ -304,7 +305,7 @@ class ParseFunction extends Function { class CreateLSParser extends Function { CreateLSParser() { this.hasName("createLSParser") and - this.getUnspecifiedType().(PointerType).getBaseType() instanceof DOMLSParserClass // returns a `DOMLSParser *`. + this.getUnspecifiedType().(PointerType).getBaseType() instanceof DomLSParserClass // returns a `DOMLSParser *`. } } From f27c2f3031837416159d1752f3bdf6f29e360831 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 11 May 2022 11:27:57 +0100 Subject: [PATCH 5/6] C++: Fix more capitalization. --- cpp/ql/src/Security/CWE/CWE-611/XXE.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql index 5f3ae65d78cc..2bb8839932ec 100644 --- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql @@ -250,10 +250,10 @@ class DomConfigurationSetParameter extends Function { * `DOMConfiguration` pointer returned by `DOMLSParser.getDomConfig` - and it * is *that* qualifier we want to transform the flow state of. */ -class DOMConfigurationSetParameterTranformer extends XXEFlowStateTranformer { +class DomConfigurationSetParameterTranformer extends XXEFlowStateTranformer { Expr newValue; - DOMConfigurationSetParameterTranformer() { + DomConfigurationSetParameterTranformer() { exists(FunctionCall getDomConfigCall, FunctionCall setParameterCall | // this is the qualifier of a call to `DOMLSParser.getDomConfig`. getDomConfigCall.getTarget() instanceof GetDomConfig and From 94e190c63af9977dfed1a33a8ed924e6de17388c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 11 May 2022 13:47:51 +0100 Subject: [PATCH 6/6] C++: getClassAndName. --- cpp/ql/src/Security/CWE/CWE-611/XXE.ql | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql index 2bb8839932ec..3ce422c89b1d 100644 --- a/cpp/ql/src/Security/CWE/CWE-611/XXE.ql +++ b/cpp/ql/src/Security/CWE/CWE-611/XXE.ql @@ -227,10 +227,7 @@ class SetFeatureTranformer extends XXEFlowStateTranformer { * The `DOMLSParser.getDomConfig` function. */ class GetDomConfig extends Function { - GetDomConfig() { - this.hasName("getDomConfig") and - this.getDeclaringType() instanceof DomLSParserClass - } + GetDomConfig() { this.getClassAndName("getDomConfig") instanceof DomLSParserClass } } /** @@ -238,8 +235,7 @@ class GetDomConfig extends Function { */ class DomConfigurationSetParameter extends Function { DomConfigurationSetParameter() { - this.hasName("setParameter") and - this.getDeclaringType().getName() = "DOMConfiguration" + this.getClassAndName("setParameter").getName() = "DOMConfiguration" } }