Merge pull request #8736 from geoffw0/xxe

C++: New query for CWE-611 / XML External Entity Expansion (XXE)
This commit is contained in:
Geoffrey White
2022-04-26 17:21:06 +01:00
committed by GitHub
9 changed files with 528 additions and 0 deletions

View File

@@ -0,0 +1,57 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Parsing untrusted XML files with a weakly configured XML parser may lead to an
XML external entity (XXE) attack. This type of attack uses external entity references
to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side
request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible
and out-of-band data retrieval techniques may allow attackers to steal sensitive data.
</p>
</overview>
<recommendation>
<p>
The easiest way to prevent XXE attacks is to disable external entity handling when
parsing untrusted data. How this is done depends on the library being used. Note that some
libraries, such as recent versions of <code>libxml</code>, disable entity expansion by default,
so unless you have explicitly enabled entity expansion, no further action needs to be taken.
</p>
</recommendation>
<example>
<p>
The following example uses the <code>Xerces-C++</code> XML parser to parse a string <code>data</code>.
If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since
the parser is constructed in its default state with <code>setDisableDefaultEntityResolution</code>
set to <code>false</code>:
</p>
<sample src="XXEBad.cpp"/>
<p>
To guard against XXE attacks, the <code>setDisableDefaultEntityResolution</code> option should be
set to <code>true</code>.
</p>
<sample src="XXEGood.cpp"/>
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.
</li>
<li>
OWASP:
<a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html">XML External Entity Prevention Cheat Sheet</a>.
</li>
<li>
Timothy Morgen:
<a href="https://research.nccgroup.com/2014/05/19/xml-schema-dtd-and-entity-attacks-a-compendium-of-known-techniques/">XML Schema, DTD, and Entity Attacks</a>.
</li>
<li>
Timur Yunusov, Alexey Osipov:
<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,209 @@
/**
* @name XML external entity expansion
* @description Parsing user-controlled XML documents and allowing expansion of
* external entity references may lead to disclosure of
* confidential data or denial of service.
* @kind path-problem
* @id cpp/external-entity-expansion
* @problem.severity warning
* @security-severity 9.1
* @precision medium
* @tags security
* external/cwe/cwe-611
*/
import cpp
import semmle.code.cpp.ir.dataflow.DataFlow
import DataFlow::PathGraph
import semmle.code.cpp.ir.IR
/**
* A flow state representing a possible configuration of an XML object.
*/
abstract class XXEFlowState extends DataFlow::FlowState {
bindingset[this]
XXEFlowState() { any() } // required characteristic predicate
}
/**
* An `Expr` that changes the configuration of an XML object, transforming the
* `XXEFlowState` that flows through it.
*/
abstract class XXEFlowStateTranformer extends Expr {
/**
* Gets the flow state that `flowstate` is transformed into.
*
* Due to limitations of the implementation the transformation defined by this
* predicate must be idempotent, that is, for any input `x` it must be that:
* ```
* transform(tranform(x)) = tranform(x)
* ```
*/
abstract XXEFlowState transform(XXEFlowState flowstate);
}
/**
* The `AbstractDOMParser` class.
*/
class AbstractDOMParserClass extends Class {
AbstractDOMParserClass() { this.hasName("AbstractDOMParser") }
}
/**
* The `XercesDOMParser` class.
*/
class XercesDOMParserClass extends Class {
XercesDOMParserClass() { this.hasName("XercesDOMParser") }
}
/**
* Gets a valid flow state for `XercesDOMParser` flow.
*
* These flow states take the form `XercesDOM-A-B`, where:
* - A is 1 if `setDisableDefaultEntityResolution` is `true`, 0 otherwise.
* - B is 1 if `setCreateEntityReferenceNodes` is `true`, 0 otherwise.
*/
predicate encodeXercesDOMFlowState(
string flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
) {
flowstate = "XercesDOM-0-0" and
disabledDefaultEntityResolution = 0 and
createEntityReferenceNodes = 0
or
flowstate = "XercesDOM-0-1" and
disabledDefaultEntityResolution = 0 and
createEntityReferenceNodes = 1
or
flowstate = "XercesDOM-1-0" and
disabledDefaultEntityResolution = 1 and
createEntityReferenceNodes = 0
or
flowstate = "XercesDOM-1-1" and
disabledDefaultEntityResolution = 1 and
createEntityReferenceNodes = 1
}
/**
* A flow state representing the configuration of a `XercesDOMParser` object.
*/
class XercesDOMParserFlowState extends XXEFlowState {
XercesDOMParserFlowState() { encodeXercesDOMFlowState(this, _, _) }
}
/**
* Flow state transformer for a call to
* `AbstractDOMParser.setDisableDefaultEntityResolution`. Transforms the flow
* state through the qualifier according to the setting in the parameter.
*/
class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
Expr newValue;
DisableDefaultEntityResolutionTranformer() {
exists(Call call, Function f |
call.getTarget() = f and
f.getDeclaringType() instanceof AbstractDOMParserClass and
f.hasName("setDisableDefaultEntityResolution") and
this = call.getQualifier() and
newValue = call.getArgument(0)
)
}
final override XXEFlowState transform(XXEFlowState flowstate) {
exists(int createEntityReferenceNodes |
encodeXercesDOMFlowState(flowstate, _, createEntityReferenceNodes) and
(
newValue.getValue().toInt() = 1 and // true
encodeXercesDOMFlowState(result, 1, createEntityReferenceNodes)
or
not newValue.getValue().toInt() = 1 and // false or unknown
encodeXercesDOMFlowState(result, 0, createEntityReferenceNodes)
)
)
}
}
/**
* Flow state transformer for a call to
* `AbstractDOMParser.setCreateEntityReferenceNodes`. Transforms the flow
* state through the qualifier according to the setting in the parameter.
*/
class CreateEntityReferenceNodesTranformer extends XXEFlowStateTranformer {
Expr newValue;
CreateEntityReferenceNodesTranformer() {
exists(Call call, Function f |
call.getTarget() = f and
f.getDeclaringType() instanceof AbstractDOMParserClass and
f.hasName("setCreateEntityReferenceNodes") and
this = call.getQualifier() and
newValue = call.getArgument(0)
)
}
final override XXEFlowState transform(XXEFlowState flowstate) {
exists(int disabledDefaultEntityResolution |
encodeXercesDOMFlowState(flowstate, disabledDefaultEntityResolution, _) and
(
newValue.getValue().toInt() = 1 and // true
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 1)
or
not newValue.getValue().toInt() = 1 and // false or unknown
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 0)
)
)
}
}
/**
* The `AbstractDOMParser.parse` method.
*/
class ParseFunction extends Function {
ParseFunction() { this.getClassAndName("parse") instanceof AbstractDOMParserClass }
}
/**
* Configuration for tracking XML objects and their states.
*/
class XXEConfiguration extends DataFlow::Configuration {
XXEConfiguration() { this = "XXEConfiguration" }
override predicate isSource(DataFlow::Node node, string flowstate) {
// source is the write on `this` of a call to the `XercesDOMParser`
// constructor.
exists(CallInstruction call |
call.getStaticCallTarget() = any(XercesDOMParserClass c).getAConstructor() and
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
call.getThisArgument() and
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
)
}
override predicate isSink(DataFlow::Node node, string flowstate) {
// sink is the read of the qualifier of a call to `parse`.
exists(Call call |
call.getTarget() instanceof ParseFunction and
call.getQualifier() = node.asConvertedExpr()
) and
flowstate instanceof XercesDOMParserFlowState and
not encodeXercesDOMFlowState(flowstate, 1, 1) // safe configuration
}
override predicate isAdditionalFlowStep(
DataFlow::Node node1, string state1, DataFlow::Node node2, string state2
) {
// create additional flow steps for `XXEFlowStateTranformer`s
state2 = node2.asConvertedExpr().(XXEFlowStateTranformer).transform(state1) and
DataFlow::simpleLocalFlowStep(node1, node2)
}
override predicate isBarrier(DataFlow::Node node, string flowstate) {
// when the flowstate is transformed at a call node, block the original
// flowstate value.
node.asConvertedExpr().(XXEFlowStateTranformer).transform(flowstate) != flowstate
}
}
from XXEConfiguration conf, DataFlow::PathNode source, DataFlow::PathNode sink
where conf.hasFlowPath(source, sink)
select sink, source, sink,
"This $@ is not configured to prevent an XML external entity (XXE) attack.", source, "XML parser"

View File

@@ -0,0 +1,4 @@
XercesDOMParser *parser = new XercesDOMParser();
parser->parse(data); // BAD (parser is not correctly configured, may expand external entity references)

View File

@@ -0,0 +1,5 @@
XercesDOMParser *parser = new XercesDOMParser();
parser->setDisableDefaultEntityResolution(true);
parser->parse(data);

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* An new query `cpp/external-entity-expansion` has been added. The query detects XML objects that are vulnerable to external entity expansion (XXE) attacks.

View File

@@ -0,0 +1,76 @@
edges
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p |
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p |
| tests.cpp:53:19:53:19 | VariableAddress [post update] | tests.cpp:55:2:55:2 | p |
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | tests.cpp:53:19:53:19 | VariableAddress [post update] |
| tests.cpp:55:2:55:2 | p | tests.cpp:56:2:56:2 | p |
| tests.cpp:56:2:56:2 | p | tests.cpp:57:2:57:2 | p |
| tests.cpp:69:19:69:19 | VariableAddress [post update] | tests.cpp:71:2:71:2 | p |
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:69:19:69:19 | VariableAddress [post update] |
| tests.cpp:71:2:71:2 | p | tests.cpp:72:2:72:2 | p |
| tests.cpp:72:2:72:2 | p | tests.cpp:73:2:73:2 | p |
| tests.cpp:73:2:73:2 | p | tests.cpp:74:2:74:2 | p |
| tests.cpp:73:2:73:2 | p | tests.cpp:74:2:74:2 | p |
| tests.cpp:74:2:74:2 | p | tests.cpp:75:2:75:2 | p |
| tests.cpp:75:2:75:2 | p | tests.cpp:76:2:76:2 | p |
| tests.cpp:76:2:76:2 | p | tests.cpp:77:2:77:2 | p |
| tests.cpp:77:2:77:2 | p | tests.cpp:78:2:78:2 | p |
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p |
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p |
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q |
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | tests.cpp:122:3:122:3 | q |
| tests.cpp:130:39:130:39 | p | tests.cpp:131:2:131:2 | p |
| tests.cpp:134:39:134:39 | p | tests.cpp:135:2:135:2 | p |
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:144:18:144:18 | q |
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:146:18:146:18 | q |
| tests.cpp:144:18:144:18 | q | tests.cpp:130:39:130:39 | p |
| tests.cpp:146:18:146:18 | q | tests.cpp:134:39:134:39 | p |
nodes
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:35:2:35:2 | p | semmle.label | p |
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:49:2:49:2 | p | semmle.label | p |
| tests.cpp:53:19:53:19 | VariableAddress [post update] | semmle.label | VariableAddress [post update] |
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:55:2:55:2 | p | semmle.label | p |
| tests.cpp:56:2:56:2 | p | semmle.label | p |
| tests.cpp:57:2:57:2 | p | semmle.label | p |
| tests.cpp:69:19:69:19 | VariableAddress [post update] | semmle.label | VariableAddress [post update] |
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:71:2:71:2 | p | semmle.label | p |
| tests.cpp:72:2:72:2 | p | semmle.label | p |
| tests.cpp:73:2:73:2 | p | semmle.label | p |
| tests.cpp:74:2:74:2 | p | semmle.label | p |
| tests.cpp:74:2:74:2 | p | semmle.label | p |
| tests.cpp:75:2:75:2 | p | semmle.label | p |
| tests.cpp:76:2:76:2 | p | semmle.label | p |
| tests.cpp:77:2:77:2 | p | semmle.label | p |
| tests.cpp:78:2:78:2 | p | semmle.label | p |
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:87:2:87:2 | p | semmle.label | p |
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:98:2:98:2 | p | semmle.label | p |
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:106:3:106:3 | q | semmle.label | q |
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:122:3:122:3 | q | semmle.label | q |
| tests.cpp:130:39:130:39 | p | semmle.label | p |
| tests.cpp:131:2:131:2 | p | semmle.label | p |
| tests.cpp:134:39:134:39 | p | semmle.label | p |
| tests.cpp:135:2:135:2 | p | semmle.label | p |
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
| tests.cpp:144:18:144:18 | q | semmle.label | q |
| tests.cpp:146:18:146:18 | q | semmle.label | q |
subpaths
#select
| tests.cpp:35:2:35:2 | p | tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:33:23:33:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:49:2:49:2 | p | tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:46:23:46:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:57:2:57:2 | p | tests.cpp:53:23:53:43 | XercesDOMParser output argument | tests.cpp:57:2:57:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:53:23:53:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:74:2:74:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:74:2:74:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:78:2:78:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:78:2:78:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:87:2:87:2 | p | tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:84:23:84:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:98:2:98:2 | p | tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:91:23:91:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:106:3:106:3 | q | tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:103:24:103:44 | XercesDOMParser output argument | XML parser |
| tests.cpp:122:3:122:3 | q | tests.cpp:118:24:118:44 | XercesDOMParser output argument | tests.cpp:122:3:122:3 | q | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:118:24:118:44 | XercesDOMParser output argument | XML parser |
| tests.cpp:131:2:131:2 | p | tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:131:2:131:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |
| tests.cpp:135:2:135:2 | p | tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:135:2:135:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |

View File

@@ -0,0 +1 @@
Security/CWE/CWE-611/XXE.ql

View File

@@ -0,0 +1,170 @@
// test cases for rule CWE-611
#include "tests.h"
// ---
class SecurityManager;
class InputSource;
class AbstractDOMParser {
public:
AbstractDOMParser();
void setDisableDefaultEntityResolution(bool); // default is false
void setCreateEntityReferenceNodes(bool); // default is true
void setSecurityManager(SecurityManager *const manager);
void parse(const InputSource &data);
};
class XercesDOMParser: public AbstractDOMParser {
public:
XercesDOMParser();
};
class LSParser: public AbstractDOMParser {
};
LSParser *createLSParser();
// ---
void test1(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->parse(data); // BAD (parser not correctly configured)
}
void test2(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(true);
p->parse(data); // GOOD
}
void test3(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(false);
p->parse(data); // BAD (parser not correctly configured)
}
void test4(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(true);
p->setCreateEntityReferenceNodes(false);
p->parse(data); // BAD (parser not correctly configured)
}
void test5(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(true);
p->setCreateEntityReferenceNodes(true);
p->parse(data); // GOOD
}
void test6(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(true);
p->parse(data); // GOOD
p->setDisableDefaultEntityResolution(false);
p->parse(data); // BAD (parser not correctly configured)
p->setDisableDefaultEntityResolution(true);
p->parse(data); // GOOD
p->setCreateEntityReferenceNodes(false);
p->parse(data); // BAD (parser not correctly configured)
p->setCreateEntityReferenceNodes(true);
p->parse(data); // GOOD
}
void test7(InputSource &data, bool cond) {
XercesDOMParser *p = new XercesDOMParser();
p->setDisableDefaultEntityResolution(cond);
p->parse(data); // BAD (parser may not be correctly configured)
}
void test8(InputSource &data, bool cond) {
XercesDOMParser *p = new XercesDOMParser();
if (cond)
{
p->setDisableDefaultEntityResolution(true);
}
p->parse(data); // BAD (parser may not be correctly configured)
}
void test9(InputSource &data) {
{
XercesDOMParser *p = new XercesDOMParser();
XercesDOMParser &q = *p;
q.parse(data); // BAD (parser not correctly configured)
}
{
XercesDOMParser *p = new XercesDOMParser();
XercesDOMParser &q = *p;
q.setDisableDefaultEntityResolution(true);
q.parse(data); // GOOD
}
{
XercesDOMParser *p = new XercesDOMParser();
XercesDOMParser &q = *p;
p->setDisableDefaultEntityResolution(true);
q.parse(data); // GOOD [FALSE POSITIVE]
}
}
void test10_doParseA(XercesDOMParser *p, InputSource &data) {
p->parse(data); // GOOD
}
void test10_doParseB(XercesDOMParser *p, InputSource &data) {
p->parse(data); // BAD (parser not correctly configured)
}
void test10_doParseC(XercesDOMParser *p, InputSource &data) {
p->parse(data); // BAD (parser may not be correctly configured)
}
void test10(InputSource &data) {
XercesDOMParser *p = new XercesDOMParser();
XercesDOMParser *q = new XercesDOMParser();
p->setDisableDefaultEntityResolution(true);
test10_doParseA(p, data);
test10_doParseB(q, data);
test10_doParseC(p, data);
test10_doParseC(q, data);
}
void test11(InputSource &data) {
LSParser *p = createLSParser();
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
}
void test12(InputSource &data) {
LSParser *p = createLSParser();
p->setDisableDefaultEntityResolution(true);
p->parse(data); // GOOD
}
LSParser *g_p1 = createLSParser();
LSParser *g_p2 = createLSParser();
InputSource *g_data;
void test13() {
g_p1->setDisableDefaultEntityResolution(true);
g_p1->parse(*g_data); // GOOD
g_p2->parse(*g_data); // BAD (parser not correctly configured) [NOT DETECTED]
}

View File

@@ -0,0 +1,2 @@
// library functions for rule CWE-611