Merge pull request #7091 from RasmusWL/port-request-without-validation

Python: Port `py/request-without-cert-validation` to use API graphs

Author: RasmusWL

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

@@ -11,17 +11,46 @@
  */
 
 import python
-import semmle.python.web.Http
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.ApiGraphs
 
-FunctionValue requestFunction() { result = Module::named("requests").attr(httpVerbLower()) }
+/**
+ * Gets a call to a method that makes an outgoing request using the `requests` module,
+ * such as `requests.get` or `requests.put`, with the specified HTTP verb `verb`
+ */
+DataFlow::CallCfgNode outgoingRequestCall(string verb) {
+  verb = HTTP::httpVerbLower() and
+  result = API::moduleImport("requests").getMember(verb).getACall()
+}
+
+/** Gets the "verfiy" argument to a outgoingRequestCall. */
+DataFlow::Node verifyArg(DataFlow::CallCfgNode call) {
+  call = outgoingRequestCall(_) and
+  result = call.getArgByName("verify")
+}
+
+/** Gets a back-reference to the verify argument `arg`. */
+private DataFlow::TypeTrackingNode verifyArgBacktracker(
+  DataFlow::TypeBackTracker t, DataFlow::Node arg
+) {
+  t.start() and
+  arg = verifyArg(_) and
+  result = arg.getALocalSource()
+  or
+  exists(DataFlow::TypeBackTracker t2 | result = verifyArgBacktracker(t2, arg).backtrack(t2, t))
+}
 
-/** requests treats None as the default and all other "falsey" values as False */
-predicate falseNotNone(Value v) { v.getDefiniteBooleanValue() = false and not v = Value::none_() }
+/** Gets a back-reference to the verify argument `arg`. */
+DataFlow::LocalSourceNode verifyArgBacktracker(DataFlow::Node arg) {
+  result = verifyArgBacktracker(DataFlow::TypeBackTracker::end(), arg)
+}
 
-from CallNode call, FunctionValue func, Value falsey, ControlFlowNode origin
+from DataFlow::CallCfgNode call, DataFlow::Node falseyOrigin, string verb
 where
-  func = requestFunction() and
-  func.getACall() = call and
-  falseNotNone(falsey) and
-  call.getArgByName("verify").pointsTo(falsey, origin)
-select call, "Call to $@ with verify=$@", func, "requests." + func.getName(), origin, "False"
+  call = outgoingRequestCall(verb) and
+  falseyOrigin = verifyArgBacktracker(verifyArg(call)) and
+  // requests treats `None` as the default and all other "falsey" values as `False`.
+  falseyOrigin.asExpr().(ImmutableLiteral).booleanValue() = false and
+  not falseyOrigin.asExpr() instanceof None
+select call, "Call to requests." + verb + " with verify=$@", falseyOrigin, "False"

python/ql/test/query-tests/Security/CWE-295-RequestWithoutValidation/RequestWithoutValidation.expected

@@ -1,5 +1,5 @@
-| make_request.py:5:1:5:48 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:2:1:2:36 | Function get | requests.get | make_request.py:5:43:5:47 | ControlFlowNode for False | False |
-| make_request.py:7:1:7:49 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:11:1:11:46 | Function post | requests.post | make_request.py:7:44:7:48 | ControlFlowNode for False | False |
-| make_request.py:12:1:12:39 | ControlFlowNode for put() | Call to $@ with verify=$@ | ../lib/requests.py:14:1:14:34 | Function put | requests.put | make_request.py:12:34:12:38 | ControlFlowNode for False | False |
-| make_request.py:28:5:28:46 | ControlFlowNode for patch() | Call to $@ with verify=$@ | ../lib/requests.py:17:1:17:36 | Function patch | requests.patch | make_request.py:30:6:30:10 | ControlFlowNode for False | False |
-| make_request.py:34:1:34:45 | ControlFlowNode for Attribute() | Call to $@ with verify=$@ | ../lib/requests.py:11:1:11:46 | Function post | requests.post | make_request.py:34:44:34:44 | ControlFlowNode for IntegerLiteral | False |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |

python/ql/test/query-tests/Security/CWE-295-RequestWithoutValidation/make_request.py

@@ -32,3 +32,7 @@ def req2(verify):
 
 #Falsey value
 requests.post('https://semmle.com', verify=0) # BAD
+
+# requests treat `None` as default value, which means it is turned on
+requests.get('https://semmle.com') # OK
+requests.get('https://semmle.com', verify=None) # OK