Skip to content

Commit c21e05a

Browse files
committed
Python: Use HTTP::Client::Request request for py/request-without-cert-validation
This is very much like the Ruby query, except we also have the origin that does the disabling. https://github.com/github/codeql/blob/976daddd36a63bf46836d141d04172e90bb4b33c/ruby/ql/src/queries/security/cwe-295/RequestWithoutValidation.ql#L18-L20
1 parent 9cb249f commit c21e05a

File tree

3 files changed

+17
-13
lines changed

3 files changed

+17
-13
lines changed

python/ql/lib/semmle/python/frameworks/Requests.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ private module Requests {
5858
) {
5959
disablingNode = this.getKeywordParameter("verify").getARhs() and
6060
argumentOrigin = this.getKeywordParameter("verify").getAValueReachingRhs() and
61+
// requests treats `None` as the default and all other "falsey" values as `False`.
6162
argumentOrigin.asExpr().(ImmutableLiteral).booleanValue() = false and
6263
not argumentOrigin.asExpr() instanceof None
6364
}

python/ql/src/Security/CWE-295/RequestWithoutValidation.ql

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ private import semmle.python.dataflow.new.DataFlow
1515
private import semmle.python.Concepts
1616
private import semmle.python.ApiGraphs
1717

18-
from API::CallNode call, DataFlow::Node falseyOrigin, string verb
18+
from
19+
HTTP::Client::Request request, DataFlow::Node disablingNode, DataFlow::Node origin, string ending
1920
where
20-
verb = HTTP::httpVerbLower() and
21-
call = API::moduleImport("requests").getMember(verb).getACall() and
22-
falseyOrigin = call.getKeywordParameter("verify").getAValueReachingRhs() and
23-
// requests treats `None` as the default and all other "falsey" values as `False`.
24-
falseyOrigin.asExpr().(ImmutableLiteral).booleanValue() = false and
25-
not falseyOrigin.asExpr() instanceof None
26-
select call, "Call to requests." + verb + " with verify=$@", falseyOrigin, "False"
21+
request.disablesCertificateValidation(disablingNode, origin) and
22+
// Showing the origin is only useful when it's a different node than the one disabling
23+
// certificate validation, for example in `requests.get(..., verify=arg)`, `arg` would
24+
// be the `disablingNode`, and the `origin` would be the place were `arg` got its
25+
// value from.
26+
if disablingNode = origin then ending = "." else ending = " by the value from $@."
27+
select request, "This request may run without certificate validation because it is $@" + ending,
28+
disablingNode, "disabled here", origin, "here"
Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
| make_request.py:5:1:5:48 | ControlFlowNode for Attribute() | Call to requests.get with verify=$@ | make_request.py:5:43:5:47 | ControlFlowNode for False | False |
2-
| make_request.py:7:1:7:49 | ControlFlowNode for Attribute() | Call to requests.post with verify=$@ | make_request.py:7:44:7:48 | ControlFlowNode for False | False |
3-
| make_request.py:12:1:12:39 | ControlFlowNode for put() | Call to requests.put with verify=$@ | make_request.py:12:34:12:38 | ControlFlowNode for False | False |
4-
| make_request.py:28:5:28:46 | ControlFlowNode for patch() | Call to requests.patch with verify=$@ | make_request.py:30:6:30:10 | ControlFlowNode for False | False |
5-
| make_request.py:34:1:34:45 | ControlFlowNode for Attribute() | Call to requests.post with verify=$@ | make_request.py:34:44:34:44 | ControlFlowNode for IntegerLiteral | False |
1+
| make_request.py:5:1:5:48 | ControlFlowNode for Attribute() | This request may run without certificate validation because it is $@. | make_request.py:5:43:5:47 | ControlFlowNode for False | disabled here | make_request.py:5:43:5:47 | ControlFlowNode for False | here |
2+
| make_request.py:7:1:7:49 | ControlFlowNode for Attribute() | This request may run without certificate validation because it is $@. | make_request.py:7:44:7:48 | ControlFlowNode for False | disabled here | make_request.py:7:44:7:48 | ControlFlowNode for False | here |
3+
| make_request.py:12:1:12:39 | ControlFlowNode for put() | This request may run without certificate validation because it is $@. | make_request.py:12:34:12:38 | ControlFlowNode for False | disabled here | make_request.py:12:34:12:38 | ControlFlowNode for False | here |
4+
| make_request.py:28:5:28:46 | ControlFlowNode for patch() | This request may run without certificate validation because it is $@ by the value from $@. | make_request.py:28:40:28:45 | ControlFlowNode for verify | disabled here | make_request.py:30:6:30:10 | ControlFlowNode for False | here |
5+
| make_request.py:34:1:34:45 | ControlFlowNode for Attribute() | This request may run without certificate validation because it is $@. | make_request.py:34:44:34:44 | ControlFlowNode for IntegerLiteral | disabled here | make_request.py:34:44:34:44 | ControlFlowNode for IntegerLiteral | here |
6+
| make_request.py:41:1:41:26 | ControlFlowNode for Attribute() | This request may run without certificate validation because it is $@. | make_request.py:41:21:41:25 | ControlFlowNode for False | disabled here | make_request.py:41:21:41:25 | ControlFlowNode for False | here |

0 commit comments

Comments
 (0)