Skip to content

Commit db616a5

Browse files
committed
python: rewrite models using subscripts
more rewrites could be done to these models for instance, I think the extra taint configuration could be removed, but here I just wanted to illustrate the benefits of the new API graph.
1 parent 0b8e908 commit db616a5

File tree

2 files changed

+36
-66
lines changed

2 files changed

+36
-66
lines changed

python/ql/src/experimental/semmle/python/frameworks/Sendgrid.qll

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ private module Sendgrid {
2626
}
2727

2828
/** Gets a reference to a `SendGridAPIClient` instance call with `send` or `post`. */
29-
private DataFlow::CallCfgNode sendgridApiSendCall() {
29+
private API::CallNode sendgridApiSendCall() {
3030
result = sendgridApiClient().getMember("send").getACall()
3131
or
3232
result =
@@ -62,7 +62,7 @@ private module Sendgrid {
6262
* * `getFrom()`'s result would be `"[email protected]"`.
6363
* * `getSubject()`'s result would be `"Sending with SendGrid is Fun"`.
6464
*/
65-
private class SendGridMail extends DataFlow::CallCfgNode, EmailSender::Range {
65+
private class SendGridMail extends API::CallNode, EmailSender::Range {
6666
SendGridMail() { this = sendgridApiSendCall() }
6767

6868
private DataFlow::CallCfgNode getMailCall() {
@@ -118,40 +118,28 @@ private module Sendgrid {
118118
or
119119
result = this.sendgridWrite("html_content")
120120
or
121-
exists(KeyValuePair content, Dict generalDict, KeyValuePair typePair, KeyValuePair valuePair |
122-
content.getKey().(StrConst).getText() = "content" and
123-
content.getValue().(List).getAnElt() = generalDict and
124-
// declare KeyValuePairs keys and values
125-
typePair.getKey().(StrConst).getText() = "type" and
126-
typePair.getValue().(StrConst).getText() = ["text/html", "text/x-amp-html"] and
127-
valuePair.getKey().(StrConst).getText() = "value" and
128-
result.asExpr() = valuePair.getValue() and
129-
// correlate generalDict with previously set KeyValuePairs
130-
generalDict.getAnItem() in [typePair, valuePair] and
131-
[this.getArg(0), this.getArgByName("request_body")].getALocalSource().asExpr() =
132-
any(Dict d | d.getAnItem() = content)
121+
exists(API::Node contentElement |
122+
contentElement =
123+
this.getKeywordParameter("request_body").getSubscript("content").getASubscript()
124+
|
125+
contentElement.getSubscript("type").getAValueReachingSink().asExpr().(StrConst).getText() =
126+
["text/html", "text/x-amp-html"] and
127+
result = contentElement.getSubscript("value").getAValueReachingSink()
133128
)
134129
or
135-
exists(KeyValuePair footer, Dict generalDict, KeyValuePair enablePair, KeyValuePair htmlPair |
136-
footer.getKey().(StrConst).getText() = ["footer", "subscription_tracking"] and
137-
footer.getValue() = generalDict and
138-
// check footer is enabled
139-
enablePair.getKey().(StrConst).getText() = "enable" and
140-
exists(enablePair.getValue().(True)) and
141-
// get html content
142-
htmlPair.getKey().(StrConst).getText() = "html" and
143-
result.asExpr() = htmlPair.getValue() and
144-
// correlate generalDict with previously set KeyValuePairs
145-
generalDict.getAnItem() in [enablePair, htmlPair] and
146-
exists(KeyValuePair k |
147-
k.getKey() =
148-
[this.getArg(0), this.getArgByName("request_body")]
149-
.getALocalSource()
150-
.asExpr()
151-
.(Dict)
152-
.getAKey() and
153-
k.getValue() = any(Dict d | d.getAKey() = footer.getKey())
154-
)
130+
exists(API::Node html |
131+
html =
132+
this.getKeywordParameter("request_body")
133+
.getSubscript("tracking_settings")
134+
.getSubscript("subscription_tracking")
135+
or
136+
html =
137+
this.getKeywordParameter("request_body")
138+
.getSubscript("mail_settings")
139+
.getSubscript("footer")
140+
|
141+
html.getSubscript("enable").getAValueReachingSink().asExpr() instanceof True and
142+
result = html.getSubscript("html").getAValueReachingSink()
155143
)
156144
}
157145

python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -101,33 +101,6 @@ module SmtpLib {
101101
)
102102
}
103103

104-
/**
105-
* Gets a message subscript write by correlating subscript's object local source with
106-
* `smtp`'s `sendmail` call 3rd argument's local source.
107-
*
108-
* Given the following example with `getSMTPSubscriptByIndex(any(SmtpLibSendMail s), "Subject")`:
109-
*
110-
* ```py
111-
* message = MIMEMultipart("alternative")
112-
* message["Subject"] = "multipart test"
113-
* server.sendmail(sender_email, receiver_email, message.as_string())
114-
* ```
115-
*
116-
* * `def` would be `message["Subject"]` (`DefinitionNode`)
117-
* * `sub` would be `message["Subject"]` (`Subscript`)
118-
* * `result` would be `"multipart test"`
119-
*/
120-
private DataFlow::Node getSmtpSubscriptByIndex(DataFlow::CallCfgNode sendCall, string index) {
121-
exists(DefinitionNode def, Subscript sub |
122-
sub = def.getNode() and
123-
DataFlow::exprNode(sub.getObject()).getALocalSource() =
124-
[sendCall.getArg(2), sendCall.getArg(2).(DataFlow::MethodCallNode).getObject()]
125-
.getALocalSource() and
126-
sub.getIndex().(StrConst).getText() = index and
127-
result.asCfgNode() = def.getValue()
128-
)
129-
}
130-
131104
/**
132105
* Gets a reference to `smtplib.SMTP_SSL().sendmail()`.
133106
*
@@ -153,7 +126,7 @@ module SmtpLib {
153126
* * `getFrom()`'s result would be `sender_email`.
154127
* * `getSubject()`'s result would be `"multipart test"`.
155128
*/
156-
private class SmtpLibSendMail extends DataFlow::CallCfgNode, EmailSender::Range {
129+
private class SmtpLibSendMail extends API::CallNode, EmailSender::Range {
157130
SmtpLibSendMail() {
158131
this = smtpConnectionInstance().getReturn().getMember("sendmail").getACall()
159132
}
@@ -163,15 +136,24 @@ module SmtpLib {
163136
override DataFlow::Node getHtmlBody() { result = getSmtpMessage(this, "html") }
164137

165138
override DataFlow::Node getTo() {
166-
result in [this.getArg(1), getSmtpSubscriptByIndex(this, "To")]
139+
result = this.getParameter(1, "to_addrs").asSink()
140+
or
141+
result = this.getMsg().getSubscript("To").asSink()
167142
}
168143

169144
override DataFlow::Node getFrom() {
170-
result in [this.getArg(0), getSmtpSubscriptByIndex(this, "From")]
145+
result = this.getParameter(0, "from_addr").asSink()
146+
or
147+
result = this.getMsg().getSubscript("From").asSink()
171148
}
172149

173-
override DataFlow::Node getSubject() {
174-
result in [this.getArg(2), getSmtpSubscriptByIndex(this, "Subject")]
150+
override DataFlow::Node getSubject() { result = this.getMsg().getSubscript("Subject").asSink() }
151+
152+
private API::Node getMsg() {
153+
result.getAValueReachableFromSource() = this.getParameter(2, "msg").asSink()
154+
or
155+
result.getMember("as_string").getReturn().getAValueReachableFromSource() =
156+
this.getParameter(2, "msg").asSink()
175157
}
176158
}
177159
}

0 commit comments

Comments
 (0)