Merge pull request github#5152 from RasmusWL/improve-pyyaml-support

Python: Improve pyyaml support

Author: yoff

python/change-notes/2021-02-10-yaml-more-loading-functions.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Improved modeling of the `PyYAML` PyPI package (imported as `yaml`) now includes `safe_load`, `unsafe_load`, and `full_load` (as well as the `..._load_all` functions). In the current version of PyYAML (5.4.1), only `safe_load` and `safe_load_all` are known to be safe from code execution exploits. Consequently, calls to the other functions are modeled as sinks of the _Deserializing untrusted input_ (`py/unsafe-deserialization`) query.

python/ql/src/Security/CWE-502/UnsafeDeserialization.qhelp

@@ -24,6 +24,9 @@ Avoid deserialization of untrusted data if at all possible.  If the
 architecture permits it then use other formats instead of serialized objects,
 for example JSON.
 </p>
+<p>
+If you need to use YAML, use the <code>yaml.safe_load</code> function.
+</p>
 </recommendation>
 
 <example>

python/ql/src/semmle/python/frameworks/Yaml.qll

@@ -29,7 +29,13 @@ private module Yaml {
      * For example, using `attr_name = "load"` will get all uses of `yaml.load`.
      */
     private DataFlow::Node yaml_attr(DataFlow::TypeTracker t, string attr_name) {
-      attr_name in ["load", "SafeLoader", "BaseLoader"] and
+      attr_name in [
+          // functions
+          "load", "load_all", "full_load", "full_load_all", "unsafe_load", "unsafe_load_all",
+          "safe_load", "safe_load_all",
+          // Classes
+          "SafeLoader", "BaseLoader"
+        ] and
       (
         t.start() and
         result = DataFlow::importNode("yaml." + attr_name)
@@ -68,13 +74,22 @@ private module Yaml {
 }
 
 /**
- * A call to `yaml.load`
+ * A call to any of the loading functions in `yaml` (`load`, `load_all`, `full_load`,
+ * `full_load_all`, `unsafe_load`, `unsafe_load_all`, `safe_load`, `safe_load_all`)
+ *
  * See https://pyyaml.org/wiki/PyYAMLDocumentation (you will have to scroll down).
  */
 private class YamlLoadCall extends Decoding::Range, DataFlow::CfgNode {
   override CallNode node;
+  string func_name;
 
-  YamlLoadCall() { node.getFunction() = Yaml::yaml::yaml_attr("load").asCfgNode() }
+  YamlLoadCall() {
+    func_name in [
+        "load", "load_all", "full_load", "full_load_all", "unsafe_load", "unsafe_load_all",
+        "safe_load", "safe_load_all"
+      ] and
+    node.getFunction() = Yaml::yaml::yaml_attr(func_name).asCfgNode()
+  }
 
   /**
    * This function was thought safe from the 5.1 release in 2017, when the default loader was changed to `FullLoader`.
@@ -84,10 +99,16 @@ private class YamlLoadCall extends Decoding::Range, DataFlow::CfgNode {
    * See https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation for more details.
    */
   override predicate mayExecuteInput() {
+    func_name in ["full_load", "full_load_all", "unsafe_load", "unsafe_load_all"]
+    or
+    func_name in ["load", "load_all"] and
     // If the `Loader` is not set to either `SafeLoader` or `BaseLoader` or not set at all,
     // then the default loader will be used, which is not safe.
-    not node.getArgByName("Loader") =
-      Yaml::yaml::yaml_attr(["SafeLoader", "BaseLoader"]).asCfgNode()
+    not exists(DataFlow::Node loader_arg |
+      loader_arg.asCfgNode() in [node.getArg(1), node.getArgByName("Loader")]
+    |
+      loader_arg = Yaml::yaml::yaml_attr(["SafeLoader", "BaseLoader"])
+    )
   }
 
   override DataFlow::Node getAnInput() { result.asCfgNode() = node.getArg(0) }

python/ql/test/experimental/library-tests/frameworks/yaml/Decoding.py

@@ -2,5 +2,15 @@
 from yaml import SafeLoader
 
 yaml.load(payload)  # $decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput
+yaml.load(payload, SafeLoader)  # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML
 yaml.load(payload, Loader=SafeLoader)  # $decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML
 yaml.load(payload, Loader=yaml.BaseLoader)  # $decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML
+
+yaml.safe_load(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML
+yaml.unsafe_load(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput
+yaml.full_load(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput
+
+yaml.load_all(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput
+yaml.safe_load_all(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML
+yaml.unsafe_load_all(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput
+yaml.full_load_all(payload) # $ decodeInput=payload decodeOutput=Attribute() decodeFormat=YAML decodeMayExecuteInput