Skip to content

Commit b31dc1e

Browse files
authored
Fix CWE queries for 295, 611, and 352 (#64)
* Update queries.py * Update prompts.py * Update MyXxeRemoteQuery.qll * Update MyInsecureTrustManagerQuery.qll * Update MyJsonpInjectionLib.qll
1 parent 09a4fe1 commit b31dc1e

5 files changed

Lines changed: 112 additions & 30 deletions

File tree

src/cwe-queries/cwe-295/MyInsecureTrustManagerQuery.qll

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import semmle.code.java.dataflow.FlowSources
55
import MyInsecureTrustManager
66
import MySources
77
import MySinks
8+
import MySummaries
9+
810

911
/**
1012
* A configuration to model the flow of an insecure `TrustManager`
@@ -13,15 +15,17 @@ import MySinks
1315
module MyInsecureTrustManagerConfig implements DataFlow::ConfigSig {
1416
predicate isSource(DataFlow::Node source) {
1517
//source instanceof InsecureTrustManagerSource
16-
isGPTDetectedSourceMethod(source.asExpr().(MethodCall).getMethod())
17-
}
18+
isGPTDetectedSource(source)
19+
}
1820

1921
predicate isSink(DataFlow::Node sink) {
2022
//sink instanceof InsecureTrustManagerSink
21-
(isGPTDetectedSinkMethodCall(sink.asExpr().(Call)) or
22-
isGPTDetectedSinkArgument(sink.asExpr().(Argument)) )
23-
and not isGuardedByInsecureFlag(this)
24-
}
23+
isGPTDetectedSink(sink)
24+
}
25+
26+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
27+
isGPTDetectedStep(n1, n2)
28+
}
2529

2630
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
2731
(isSink(node) or isAdditionalFlowStep(node, _)) and

src/cwe-queries/cwe-352/MyJsonpInjectionLib.qll

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,22 @@ class JsonpBuilderExpr extends AddExpr {
3535

3636
/** A data flow configuration tracing flow from threat model sources to jsonp function name. */
3737
module MyThreatModelFlowConfig implements DataFlow::ConfigSig {
38-
predicate isSource(DataFlow::Node src) { isGPTDetectedSource(src) }
38+
predicate isSource(DataFlow::Node source) {
39+
isGPTDetectedSource(source)
40+
}
3941

4042
predicate isSink(DataFlow::Node sink) {
41-
exists(JsonpBuilderExpr jhe | jhe.getFunctionName() = sink.asExpr())
43+
isGPTDetectedSink(sink)
4244
}
4345
}
4446

4547
module MyThreatModelFlow = DataFlow::Global<MyThreatModelFlowConfig>;
4648

4749
/** A data flow configuration tracing flow from json data into the argument `json` of JSONP-like string `someFunctionName + "(" + json + ")"`. */
4850
module JsonDataFlowConfig implements DataFlow::ConfigSig {
49-
predicate isSource(DataFlow::Node src) { src instanceof JsonStringSource }
51+
predicate isSource(DataFlow::Node source) {
52+
isGPTDetectedSource(source)
53+
}
5054

5155
predicate isSink(DataFlow::Node sink) {
5256
exists(JsonpBuilderExpr jhe | jhe.getJsonExpr() = sink.asExpr())
@@ -57,15 +61,13 @@ module JsonDataFlow = DataFlow::Global<JsonDataFlowConfig>;
5761

5862
/** Taint-tracking configuration tracing flow from probable jsonp data with a user-controlled function name to an outgoing HTTP entity. */
5963
module MyJsonpInjectionFlowConfig implements DataFlow::ConfigSig {
60-
predicate isSource(DataFlow::Node src) {
61-
exists(JsonpBuilderExpr jhe |
62-
jhe = src.asExpr() and
63-
JsonDataFlow::flowTo(DataFlow::exprNode(jhe.getJsonExpr())) and
64-
MyThreatModelFlow::flowTo(DataFlow::exprNode(jhe.getFunctionName()))
65-
)
64+
predicate isSource(DataFlow::Node source) {
65+
isGPTDetectedSource(source)
6666
}
6767

68-
predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
68+
predicate isSink(DataFlow::Node sink) {
69+
isGPTDetectedSink(sink)
70+
}
6971
}
7072

7173
module MyJsonpInjectionFlow = TaintTracking::Global<MyJsonpInjectionFlowConfig>;
@@ -77,7 +79,7 @@ module MyRequestResponseFlowConfig implements DataFlow::ConfigSig {
7779
}
7880

7981
predicate isSink(DataFlow::Node sink) {
80-
sink instanceof XssSink and isGPTDetectedSink(sink)
82+
isGPTDetectedSink(sink)
8183
}
8284

8385
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {

src/cwe-queries/cwe-611/MyXxeRemoteQuery.qll

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,28 @@ private import semmle.code.java.dataflow.TaintTracking
66
private import MyXxeQuery
77
import MySources
88
import MySinks
9+
import MySummaries
910

1011
/**
1112
* A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion.
1213
*/
1314
module MyXxeConfig implements DataFlow::ConfigSig {
14-
predicate isSource(DataFlow::Node src) {
15+
predicate isSource(DataFlow::Node source) {
1516
//src instanceof ThreatModelFlowSource
16-
isGPTDetectedSourceMethod(source.asExpr().(MethodCall).getMethod())
17-
}
17+
isGPTDetectedSource(source)
18+
}
1819

1920
predicate isSink(DataFlow::Node sink) {
2021
//sink instanceof XxeSink
21-
isGPTDetectedSinkMethodCall(sink.asExpr().(Call)) or
22-
23-
// an argument to a method call
24-
isGPTDetectedSinkArgument(sink.asExpr().(Argument))
25-
}
22+
isGPTDetectedSink(sink)
23+
}
2624

2725
predicate isBarrier(DataFlow::Node sanitizer) {
2826
sanitizer instanceof XxeSanitizer
29-
}
27+
}
3028

3129
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
32-
any(XxeAdditionalTaintStep s).step(n1, n2)
30+
isGPTDetectedStep(n1, n2)
3331
}
3432
}
3533

src/prompts.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@
133133
"918": "Server-Side Request Forgery occurs when untrusted input controls the target of an outgoing HTTP or other protocol request. Watch for user input flowing into URL constructors, HTTP client execute/connect methods, or SSRF-related libraries without validation.",
134134
"502": "Be cautious of calls to deserialization methods like `readObject()` or `deserialize()` when passed data from untrusted sources. Attackers may craft malicious object graphs or gadget chains to trigger unexpected behavior or even remote code execution. Check if class allowlisting or validation is in place. Avoid deserializing directly from network input or unvalidated byte arrays.",
135135
"807": "Pay special attention to cases where user-controlled input is directly used in permission checks (e.g., permission strings or resource identifiers). Focus on whether permission checks (such as Subject.isPermitted or similar APIs) rely on tainted or untrusted data, which may allow privilege escalation or unauthorized access.",
136-
"352": "Check if the JSONP callback parameter is validated or restricted. Unchecked callback parameters may allow attackers to inject arbitrary JavaScript, leading to CSRF or data theft."
136+
"352": "Check if the JSONP callback parameter is validated or restricted. Unchecked callback parameters may allow attackers to inject arbitrary JavaScript, leading to CSRF or data theft.",
137+
"611": "Check if a default entity resolver is enabled, and if there is a Document Type Definition (DTD). The DTD may include arbitrary HTTP requests that the server may execute. Be careful with deserialization of XML-derived objects.",
138+
"295": "If certificate pinning is being used, ensure that all relevant properties of the certificate are fully validated before the certificate is pinned, including the hostname. Always verify the full certificate chain."
137139
}
138140

139141
SNIPPET_CONTEXT_SIZE = 4

src/queries.py

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@
635635
"queries": [
636636
"cwe-queries/cwe-352/cwe-352wLLM.ql",
637637
"cwe-queries/cwe-352/MyJsonpInjectionLib.qll",
638-
"cwe-queries/cwe-352/MyJsonStringLib.qll",
638+
"cwe-queries/cwe-352/MyJsonStringLib.qll",
639639
],
640640
"prompts": {
641641
"cwe_id": "CWE-352",
@@ -672,7 +672,83 @@
672672
]
673673
}
674674
},
675-
675+
"cwe-611wLLM": {
676+
"name": "cwe-611wLLM",
677+
"type": "cwe-query",
678+
"cwe_id": "611",
679+
"cwe_id_short": "611",
680+
"cwe_id_tag": "CWE-611",
681+
"desc": "Improper Restriction of XML External Entity Reference",
682+
"queries": [
683+
"cwe-queries/cwe-611/XXE.ql",
684+
"cwe-queries/cwe-611/MyXxeRemoteQuery.qll",
685+
"cwe-queries/cwe-611/MyXxeQuery.qll",
686+
"cwe-queries/cwe-611/MyXxe.qll"
687+
],
688+
"prompts": {
689+
"cwe_id": "CWE-611",
690+
"desc": "Improper Restriction of XML External Entity Reference",
691+
"long_desc": """\
692+
XML documents optionally contain a Document Type Definition (DTD), which, among other features, enables the definition of XML entities. It is possible to define an entity by providing a substitution string in the form of a URI. The XML parser can access the contents of this URI and embed these contents back into the XML document for further processing. \
693+
By submitting an XML file that defines an external entity with a file:// URI, an attacker can cause the processing application to read the contents of a local file. For example, a URI such as "file:///c:/winnt/win.ini" designates (in Windows) the file C:\Winnt\win.ini, or file:///etc/passwd designates the password file in Unix-based systems. Using URIs with other schemes such as http://, the attacker can force the application to make outgoing requests to servers that the attacker cannot reach directly, which can be used to bypass firewall restrictions or hide the source of attacks such as port scanning.\
694+
Once the content of the URI is read, it is fed back into the application that is processing the XML. This application may echo back the data (e.g. in an error message), thereby exposing the file contents.""",
695+
"examples": [
696+
{
697+
"package": "javax.xml.transform",
698+
"class": "DefaultDDFFileValidator",
699+
"method": "validate",
700+
"signature": "void validate(Source xmlToValidate)",
701+
"sink_args": [],
702+
"type": "source"
703+
},
704+
{
705+
"package": "java.xml.parsers.DocumentBuilderFactory",
706+
"class": "DOMWalker",
707+
"method": "",
708+
"signature": "Object readObject()",
709+
"sink_args": [],
710+
"type": "sink"
711+
},
712+
]
713+
}
714+
},
715+
"cwe-295wLLM": {
716+
"name": "cwe-295wLLM",
717+
"type": "cwe-query",
718+
"cwe_id": "295",
719+
"cwe_id_short": "295",
720+
"cwe_id_tag": "CWE-295",
721+
"desc": "Improper Certificate Validation",
722+
"queries": [
723+
"cwe-queries/cwe-295/InsecureTrustManager.ql",
724+
"cwe-queries/cwe-295/MyInsecureTrustManager.qll",
725+
"cwe-queries/cwe-295/MyInsecureTrustManagerQuery.qll",
726+
],
727+
"prompts": {
728+
"cwe_id": "CWE-295",
729+
"desc": "Improper Certificate Validation",
730+
"long_desc": """\
731+
When a certificate is invalid or malicious, it might allow an attacker to spoof a trusted entity by interfering in the communication path between the host and client. The product might connect to a malicious host while believing it is a trusted host, or the product might be deceived into accepting spoofed data that appears to originate from a trusted host.""",
732+
"examples": [
733+
{
734+
"package": "org.keycloak.authentication.AuthenticationFlowContext",
735+
"class": "ValidateX509CertificateUsername",
736+
"method": "authenticate",
737+
"signature": "void authenticate(AuthenticationFlowContext context)",
738+
"sink_args": [],
739+
"type": "source"
740+
},
741+
{
742+
"package": "com.tigervnc.rfb",
743+
"class": "CSecurityTLS",
744+
"method": "checkServerTrusted",
745+
"signature": "checkServerTrusted(X509Certificate[] chain, String authType)",
746+
"sink_args": ["chain", "authType"],
747+
"type": "sink"
748+
},
749+
]
750+
}
751+
},
676752

677753
"fetch_external_apis": {
678754
"name": "fetch_external_apis",

0 commit comments

Comments
 (0)