Merge pull request #2644 from RasmusWL/python-add-deprecated-keyword

Python: Add deprecated keyword to deprecated functions

Author: tausbn

python/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql

@@ -1,31 +1,30 @@
 /**
  * @name Use of a broken or weak cryptographic algorithm
  * @description Using broken or weak cryptographic algorithms can compromise security.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision high
  * @id py/weak-cryptographic-algorithm
  * @tags security
  *       external/cwe/cwe-327
  */
+
 import python
+import semmle.python.security.Paths
 import semmle.python.security.SensitiveData
 import semmle.python.security.Crypto
 
 class BrokenCryptoConfiguration extends TaintTracking::Configuration {
-
     BrokenCryptoConfiguration() { this = "Broken crypto configuration" }
 
-    override predicate isSource(TaintTracking::Source source) { source instanceof SensitiveDataSource }
-
-    override predicate isSink(TaintTracking::Sink sink) {
-        sink instanceof WeakCryptoSink
+    override predicate isSource(TaintTracking::Source source) {
+        source instanceof SensitiveDataSource
     }
 
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof WeakCryptoSink }
 }
 
-
-from BrokenCryptoConfiguration config, SensitiveDataSource src, WeakCryptoSink sink
-where config.hasFlow(src, sink)
-
-select sink, "Sensitive data from $@ is used in a broken or weak cryptographic algorithm.", src , src.toString()
+from BrokenCryptoConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where config.hasFlowPath(src, sink)
+select sink.getSink(), src, sink, "$@ is used in a broken or weak cryptographic algorithm.",
+    src.getSource(), "Sensitive data"

python/ql/src/Security/CWE-798/HardcodedCredentials.ql

@@ -1,7 +1,7 @@
 /**
  * @name Hard-coded credentials
  * @description Credentials are hard coded in the source code of the application.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision medium
  * @id py/hardcoded-credentials
@@ -12,22 +12,20 @@
  */
 
 import python
+import semmle.python.security.Paths
 import semmle.python.security.TaintTracking
 import semmle.python.filters.Tests
 
 class HardcodedValue extends TaintKind {
-
-    HardcodedValue() {
-        this = "hard coded value"
-    }
-
+    HardcodedValue() { this = "hard coded value" }
 }
 
 bindingset[char, fraction]
 predicate fewer_characters_than(StrConst str, string char, float fraction) {
     exists(string text, int chars |
         text = str.getText() and
-        chars = count(int i | text.charAt(i) = char) |
+        chars = count(int i | text.charAt(i) = char)
+    |
         /* Allow one character */
         chars = 1 or
         chars < text.length() * fraction
@@ -46,118 +44,88 @@ predicate possible_reflective_name(string name) {
     exists(Object::builtin(name))
 }
 
-int char_count(StrConst str) {
-    result = count(string c | c = str.getText().charAt(_))
-}
+int char_count(StrConst str) { result = count(string c | c = str.getText().charAt(_)) }
 
-predicate capitalized_word(StrConst str) {
-    str.getText().regexpMatch("[A-Z][a-z]+")
-}
+predicate capitalized_word(StrConst str) { str.getText().regexpMatch("[A-Z][a-z]+") }
 
-predicate format_string(StrConst str) {
-    str.getText().matches("%{%}%")
-}
+predicate format_string(StrConst str) { str.getText().matches("%{%}%") }
 
 predicate maybeCredential(ControlFlowNode f) {
     /* A string that is not too short and unlikely to be text or an identifier. */
-    exists(StrConst str |
-        str = f.getNode() |
+    exists(StrConst str | str = f.getNode() |
         /* At least 10 characters */
         str.getText().length() > 9 and
         /* Not too much whitespace */
         fewer_characters_than(str, " ", 0.05) and
         /* or underscores */
         fewer_characters_than(str, "_", 0.2) and
         /* Not too repetitive */
-        exists(int chars |
-            chars = char_count(str) |
+        exists(int chars | chars = char_count(str) |
             chars > 15 or
-            chars*3 > str.getText().length()*2
+            chars * 3 > str.getText().length() * 2
         ) and
         not possible_reflective_name(str.getText()) and
         not capitalized_word(str) and
         not format_string(str)
     )
     or
     /* Or, an integer with over 32 bits */
-    exists(IntegerLiteral lit |
-        f.getNode() = lit
-        |
+    exists(IntegerLiteral lit | f.getNode() = lit |
         not exists(lit.getValue()) and
         /* Not a set of flags or round number */
         not lit.getN().matches("%00%")
     )
 }
 
 class HardcodedValueSource extends TaintSource {
+    HardcodedValueSource() { maybeCredential(this) }
 
-    HardcodedValueSource() {
-        maybeCredential(this)
-    }
-
-    override predicate isSourceOf(TaintKind kind) {
-        kind instanceof HardcodedValue
-    }
-
+    override predicate isSourceOf(TaintKind kind) { kind instanceof HardcodedValue }
 }
 
 class CredentialSink extends TaintSink {
-
     CredentialSink() {
         exists(string name |
             name.regexpMatch(getACredentialRegex()) and
-            not name.suffix(name.length()-4) = "file"
-            |
+            not name.suffix(name.length() - 4) = "file"
+        |
             any(FunctionObject func).getNamedArgumentForCall(_, name) = this
             or
-            exists(Keyword k |
-                k.getArg() = name and k.getValue().getAFlowNode() = this
-            )
+            exists(Keyword k | k.getArg() = name and k.getValue().getAFlowNode() = this)
             or
-            exists(CompareNode cmp, NameNode n |
-                n.getId() = name
-                |
+            exists(CompareNode cmp, NameNode n | n.getId() = name |
                 cmp.operands(this, any(Eq eq), n)
                 or
                 cmp.operands(n, any(Eq eq), this)
             )
         )
     }
 
-
-    override predicate sinks(TaintKind kind) {
-        kind instanceof HardcodedValue
-    }
-
+    override predicate sinks(TaintKind kind) { kind instanceof HardcodedValue }
 }
 
 /**
-  * Gets a regular expression for matching names of locations (variables, parameters, keys) that
-  * indicate the value being held is a credential.
-  */
+ * Gets a regular expression for matching names of locations (variables, parameters, keys) that
+ * indicate the value being held is a credential.
+ */
 private string getACredentialRegex() {
-  result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
-  result = "(?i).*(puid|username|userid).*" or
-  result = "(?i).*(cert)(?!.*(format|name)).*"
+    result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
+    result = "(?i).*(puid|username|userid).*" or
+    result = "(?i).*(cert)(?!.*(format|name)).*"
 }
 
 class HardcodedCredentialsConfiguration extends TaintTracking::Configuration {
-
     HardcodedCredentialsConfiguration() { this = "Hardcoded coredentials configuration" }
 
-    override predicate isSource(TaintTracking::Source source) { source instanceof HardcodedValueSource }
-
-    override predicate isSink(TaintTracking::Sink sink) {
-        sink instanceof CredentialSink
+    override predicate isSource(TaintTracking::Source source) {
+        source instanceof HardcodedValueSource
     }
 
+    override predicate isSink(TaintTracking::Sink sink) { sink instanceof CredentialSink }
 }
 
-
-
-from HardcodedCredentialsConfiguration config, TaintSource src, TaintSink sink
-
-where config.hasFlow(src, sink) and
-not any(TestScope test).contains(src.(ControlFlowNode).getNode())
-
-select sink, "Use of hardcoded credentials from $@.", src, src.toString()
+from HardcodedCredentialsConfiguration config, TaintedPathSource src, TaintedPathSink sink
+where
+    config.hasFlowPath(src, sink) and
+    not any(TestScope test).contains(src.getAstNode())
+select sink.getSink(), src, sink, "Use of $@.", src.getSource(), "hardcoded credentials"

python/ql/src/Statements/ModificationOfLocals.ql

@@ -25,7 +25,7 @@ Object aFunctionLocalsObject() {
 
 
 predicate modification_of_locals(ControlFlowNode f) {
-    f.(SubscriptNode).getValue().refersTo(aFunctionLocalsObject()) and (f.isStore() or f.isDelete())
+    f.(SubscriptNode).getObject().refersTo(aFunctionLocalsObject()) and (f.isStore() or f.isDelete())
     or
     exists(string mname, AttrNode attr |
         attr = f.(CallNode).getFunction() and

python/ql/src/Statements/RedundantAssignment.ql

@@ -65,7 +65,7 @@ predicate same_attribute(Attribute a1, Attribute a2) {
 
 int pyflakes_commented_line(File file) {
     exists(Comment c | c.getText().toLowerCase().matches("%pyflakes%") |
-        c.getLocation().hasLocationInfo(file.getName(), result, _, _, _)
+        c.getLocation().hasLocationInfo(file.getAbsolutePath(), result, _, _, _)
     )
 }
 

python/ql/src/Variables/UndefinedExport.ql

@@ -30,7 +30,7 @@ predicate mutates_globals(ModuleValue m) {
     |
         exists(AttrNode attr | attr.getObject() = globals)
         or
-        exists(SubscriptNode sub | sub.getValue() = globals and sub.isStore())
+        exists(SubscriptNode sub | sub.getObject() = globals and sub.isStore())
     )
     or
     exists(Value enum_convert, ClassValue enum_class |

python/ql/src/analysis/Sanity.ql

@@ -206,8 +206,8 @@ predicate file_sanity(string clsname, string problem, string what) {
     exists(File file, Folder folder |
         clsname = file.getAQlClass() and
         problem = "has same name as a folder" and
-        what = file.getName() and
-        what = folder.getName()
+        what = file.getAbsolutePath() and
+        what = folder.getAbsolutePath()
     ) or
     exists(Container f |
         clsname = f.getAQlClass() and

python/ql/src/external/DuplicateBlock.ql

@@ -21,14 +21,14 @@ predicate sorted_by_location(DuplicateBlock x, DuplicateBlock y) {
     if x.sourceFile() = y.sourceFile() then
         x.sourceStartLine() < y.sourceStartLine()
     else
-        x.sourceFile().getName() < y.sourceFile().getName()
+        x.sourceFile().getAbsolutePath() < y.sourceFile().getAbsolutePath()
 }
 
 from DuplicateBlock d, DuplicateBlock other
 where d.sourceLines() > 10 and
       other.getEquivalenceClass() = d.getEquivalenceClass() and
       sorted_by_location(other, d)
-select 
-   d, 
+select
+   d,
    "Duplicate code: " + d.sourceLines() + " lines are duplicated at " +
    other.sourceFile().getShortName() + ":" + other.sourceStartLine().toString()

python/ql/src/semmle/python/Files.qll

@@ -9,17 +9,17 @@ class File extends Container {
     }
 
     /** DEPRECATED: Use `getAbsolutePath` instead. */
-    override string getName() {
-        files(this, result, _, _, _)
+    deprecated override string getName() {
+        result = this.getAbsolutePath()
     }
 
     /** DEPRECATED: Use `getAbsolutePath` instead. */
-    string getFullName() {
-        result = getName()
+    deprecated string getFullName() {
+        result = this.getAbsolutePath()
     }
 
     predicate hasLocationInfo(string filepath, int bl, int bc, int el, int ec) {
-        this.getName() = filepath and bl = 0 and bc = 0 and el = 0 and ec = 0
+        this.getAbsolutePath() = filepath and bl = 0 and bc = 0 and el = 0 and ec = 0
     }
 
     /** Whether this file is a source code file. */
@@ -97,17 +97,17 @@ class Folder extends Container {
     }
 
     /** DEPRECATED: Use `getAbsolutePath` instead. */
-    override string getName() {
-        folders(this, result, _)
+    deprecated override string getName() {
+        result = this.getAbsolutePath()
     }
 
     /** DEPRECATED: Use `getBaseName` instead. */
-    string getSimple() {
+    deprecated string getSimple() {
         folders(this, _, result)
     }
 
     predicate hasLocationInfo(string filepath, int bl, int bc, int el, int ec) {
-        this.getName() = filepath and bl = 0 and bc = 0 and el = 0 and ec = 0
+        this.getAbsolutePath() = filepath and bl = 0 and bc = 0 and el = 0 and ec = 0
     }
 
     override string getAbsolutePath() {
@@ -427,11 +427,11 @@ class Location extends @location {
     }
 
     string toString() {
-        result = this.getPath().getName() + ":" + this.getStartLine().toString()
+        result = this.getPath().getAbsolutePath() + ":" + this.getStartLine().toString()
     }
 
     predicate hasLocationInfo(string filepath, int bl, int bc, int el, int ec) {
-        exists(File f | f.getName() = filepath |
+        exists(File f | f.getAbsolutePath() = filepath |
             locations_default(this, f, bl, bc, el, ec)
             or
             exists(Module m | m.getFile() = f |
@@ -445,7 +445,7 @@ class Location extends @location {
 class Line extends @py_line {
 
     predicate hasLocationInfo(string filepath, int bl, int bc, int el, int ec) {
-        exists(Module m | m.getFile().getName() = filepath and
+        exists(Module m | m.getFile().getAbsolutePath() = filepath and
             el = bl and bc = 1 and
             py_line_lengths(this, m, bl, ec))
     }

python/ql/src/semmle/python/Flow.qll

@@ -606,7 +606,7 @@ class SubscriptNode extends ControlFlowNode {
 
     /** DEPRECATED: Use `getObject()` instead.
      * This will be formally deprecated before the end 2018 and removed in 2019.*/
-    ControlFlowNode getValue() {
+    deprecated ControlFlowNode getValue() {
         exists(Subscript s | this.getNode() = s and s.getObject() = result.getNode() and
         result.getBasicBlock().dominates(this.getBasicBlock()))
     }

python/ql/src/semmle/python/dataflow/Configuration.qll

@@ -123,7 +123,7 @@ module TaintTracking {
         /* Old query API */
 
         /* deprecated */
-        predicate hasFlow(Source src, Sink sink) {
+        deprecated predicate hasFlow(Source src, Sink sink) {
             exists(PathSource psrc, PathSink psink |
                 this.hasFlowPath(psrc, psink) and
                 src = psrc.getNode().asCfgNode() and