Skip to content

Commit 1d00730

Browse files
committed
Python: Allow http[s]:// prefix for SSRF
1 parent 8d9a797 commit 1d00730

File tree

3 files changed

+156
-21
lines changed

3 files changed

+156
-21
lines changed

python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryCustomizations.qll

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,29 +83,85 @@ module ServerSideRequestForgery {
8383
/**
8484
* A string construction (concat, format, f-string) where the left side is not
8585
* user-controlled.
86+
*
87+
* For all of these cases, we try to allow `http://` or `https://` on the left side
88+
* since that will still allow full URL control.
8689
*/
8790
class StringConstructioneAsFullUrlControlSanitizer extends FullUrlControlSanitizer {
8891
StringConstructioneAsFullUrlControlSanitizer() {
8992
// string concat
9093
exists(BinaryExprNode add |
9194
add.getOp() instanceof Add and
92-
add.getRight() = this.asCfgNode()
95+
add.getRight() = this.asCfgNode() and
96+
not add.getLeft().getNode().(StrConst).getText().toLowerCase() in ["http://", "https://"]
9397
)
9498
or
9599
// % formatting
96100
exists(BinaryExprNode fmt |
97101
fmt.getOp() instanceof Mod and
98-
fmt.getRight() = this.asCfgNode()
102+
fmt.getRight() = this.asCfgNode() and
103+
// detecting %-formatting is not super easy, so we simplify it to only handle
104+
// when there is a **single** substitution going on.
105+
not fmt.getLeft().getNode().(StrConst).getText().regexpMatch("^(?i)https?://%s[^%]*$")
99106
)
100107
or
101108
// arguments to a format call
102-
exists(DataFlow::MethodCallNode call |
109+
exists(DataFlow::MethodCallNode call, string httpPrefixRe |
110+
httpPrefixRe = "^(?i)https?://(?:(\\{\\})|\\{([0-9]+)\\}|\\{([^0-9].*)\\}).*$"
111+
|
103112
call.getMethodName() = "format" and
104-
this in [call.getArg(_), call.getArgByName(_)]
113+
(
114+
if call.getObject().asExpr().(StrConst).getText().regexpMatch(httpPrefixRe)
115+
then
116+
exists(string text | text = call.getObject().asExpr().(StrConst).getText() |
117+
// `http://{}...`
118+
exists(text.regexpCapture(httpPrefixRe, 1)) and
119+
this in [call.getArg(any(int i | i >= 1)), call.getArgByName(_)]
120+
or
121+
// `http://{123}...`
122+
exists(int safeArgIndex | safeArgIndex = text.regexpCapture(httpPrefixRe, 2).toInt() |
123+
this in [call.getArg(any(int i | i != safeArgIndex)), call.getArgByName(_)]
124+
)
125+
or
126+
// `http://{abc}...`
127+
exists(string safeArgName | safeArgName = text.regexpCapture(httpPrefixRe, 3) |
128+
this in [call.getArg(_), call.getArgByName(any(string s | s != safeArgName))]
129+
)
130+
)
131+
else this in [call.getArg(_), call.getArgByName(_)]
132+
)
105133
)
106134
or
107135
// f-string
108-
exists(Fstring fstring | fstring.getValue(any(int i | i > 0)) = this.asExpr())
136+
exists(Fstring fstring |
137+
if fstring.getValue(0).(StrConst).getText().toLowerCase() in ["http://", "https://"]
138+
then fstring.getValue(any(int i | i >= 2)) = this.asExpr()
139+
else fstring.getValue(any(int i | i >= 1)) = this.asExpr()
140+
)
109141
}
110142
}
111143
}
144+
145+
predicate debug(Location loc, DataFlow::MethodCallNode call, string text, DataFlow::Node safe) {
146+
loc = call.getLocation() and
147+
call.getMethodName() = "format" and
148+
text = call.getObject().asExpr().(StrConst).getText() and
149+
exists(string httpPrefixRe |
150+
httpPrefixRe = "^(?i)https?://(?:(\\{\\})|\\{([0-9]+)\\}|\\{([^0-9].*)\\}).*$" and
151+
text.regexpMatch(httpPrefixRe)
152+
|
153+
// `http://{123}...`
154+
exists(int safeArgIndex | safeArgIndex = text.regexpCapture(httpPrefixRe, 2).toInt() |
155+
safe = call.getArg(safeArgIndex)
156+
)
157+
or
158+
// `http://{abc}...`
159+
exists(string safeArgName | safeArgName = text.regexpCapture(httpPrefixRe, 3) |
160+
safe = call.getArgByName(safeArgName)
161+
)
162+
or
163+
// `http://{}...`
164+
exists(text.regexpCapture(httpPrefixRe, 1)) and
165+
safe = call.getArg(0)
166+
)
167+
}

0 commit comments

Comments
 (0)