Merge pull request github#3837 from geoffw0/qldoc5

C++/Java: Update QLDoc and terminology in Encryption.qll

Author: MathiasVP

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

@@ -18,7 +18,7 @@ abstract class InsecureCryptoSpec extends Locatable {
 }
 
 Function getAnInsecureFunction() {
-  result.getName().regexpMatch(algorithmBlacklistRegex()) and
+  result.getName().regexpMatch(getInsecureAlgorithmRegex()) and
   exists(result.getACallToThisFunction())
 }
 
@@ -33,7 +33,7 @@ class InsecureFunctionCall extends InsecureCryptoSpec, FunctionCall {
 }
 
 Macro getAnInsecureMacro() {
-  result.getName().regexpMatch(algorithmBlacklistRegex()) and
+  result.getName().regexpMatch(getInsecureAlgorithmRegex()) and
   exists(result.getAnInvocation())
 }
 

cpp/ql/src/semmle/code/cpp/security/Encryption.qll

@@ -1,38 +1,50 @@
-// Common predicates relating to encryption in C and C++
+/**
+ * Provides predicates relating to encryption in C and C++.
+ */
+
 import cpp
 
-/** A blacklist of algorithms that are known to be insecure */
-string algorithmBlacklist() {
+/**
+ * Gets the name of an algorithm that is known to be insecure.
+ */
+string getAnInsecureAlgorithmName() {
   result = "DES" or
   result = "RC2" or
   result = "RC4" or
   result = "RC5" or
   result = "ARCFOUR" // a variant of RC4
 }
 
-// these are only bad if they're being used for encryption, and it's
-// hard to know when that's happening
-string hashAlgorithmBlacklist() {
+/**
+ * Gets the name of a hash algorithm that is insecure if it is being used for
+ * encryption (but it is hard to know when that is happening).
+ */
+string getAnInsecureHashAlgorithmName() {
   result = "SHA1" or
   result = "MD5"
 }
 
-/** A regex for matching strings that look like they contain a blacklisted algorithm */
-string algorithmBlacklistRegex() {
+/**
+ * Gets the regular expression used for matching strings that look like they
+ * contain an algorithm that is known to be insecure.
+ */
+string getInsecureAlgorithmRegex() {
   result =
     // algorithms usually appear in names surrounded by characters that are not
     // alphabetical characters in the same case. This handles the upper and lower
     // case cases
-    "(^|.*[^A-Z])(" + strictconcat(algorithmBlacklist(), "|") + ")([^A-Z].*|$)" + "|" +
+    "(^|.*[^A-Z])(" + strictconcat(getAnInsecureAlgorithmName(), "|") + ")([^A-Z].*|$)" + "|" +
       // for lowercase, we want to be careful to avoid being confused by camelCase
       // hence we require two preceding uppercase letters to be sure of a case switch,
       // or a preceding non-alphabetic character
-      "(^|.*[A-Z]{2}|.*[^a-zA-Z])(" + strictconcat(algorithmBlacklist().toLowerCase(), "|") +
+      "(^|.*[A-Z]{2}|.*[^a-zA-Z])(" + strictconcat(getAnInsecureAlgorithmName().toLowerCase(), "|") +
       ")([^a-z].*|$)"
 }
 
-/** A whitelist of algorithms that are known to be secure */
-string algorithmWhitelist() {
+/**
+ * Gets the name of an algorithm that is known to be secure.
+ */
+string getASecureAlgorithmName() {
   result = "RSA" or
   result = "SHA256" or
   result = "CCM" or
@@ -42,17 +54,46 @@ string algorithmWhitelist() {
   result = "ECIES"
 }
 
-/** A regex for matching strings that look like they contain a whitelisted algorithm */
-string algorithmWhitelistRegex() {
-  // The implementation of this is a duplicate of algorithmBlacklistRegex, as it isn't
-  // possible to have string -> string functions at the moment
+/**
+ * Gets a regular expression for matching strings that look like they
+ * contain an algorithm that is known to be secure.
+ */
+string getSecureAlgorithmRegex() {
   // algorithms usually appear in names surrounded by characters that are not
   // alphabetical characters in the same case. This handles the upper and lower
   // case cases
-  result = "(^|.*[^A-Z])" + algorithmWhitelist() + "([^A-Z].*|$)"
+  result = "(^|.*[^A-Z])" + getASecureAlgorithmName() + "([^A-Z].*|$)"
   or
   // for lowercase, we want to be careful to avoid being confused by camelCase
-  // hence we require two preceding uppercase letters to be sure of a case switch,
-  // or a preceding non-alphabetic character
-  result = "(^|.*[A-Z]{2}|.*[^a-zA-Z])" + algorithmWhitelist().toLowerCase() + "([^a-z].*|$)"
+  // hence we require two preceding uppercase letters to be sure of a case
+  // switch, or a preceding non-alphabetic character
+  result = "(^|.*[A-Z]{2}|.*[^a-zA-Z])" + getASecureAlgorithmName().toLowerCase() + "([^a-z].*|$)"
 }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getAnInsecureAlgorithmName()`
+ * instead.
+ */
+deprecated string algorithmBlacklist() { result = getAnInsecureAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use
+ * `getAnInsecureHashAlgorithmName()` instead.
+ */
+deprecated string hashAlgorithmBlacklist() { result = getAnInsecureHashAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getInsecureAlgorithmRegex()` instead.
+ */
+deprecated string algorithmBlacklistRegex() { result = getInsecureAlgorithmRegex() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getASecureAlgorithmName()`
+ * instead.
+ */
+deprecated string algorithmWhitelist() { result = getASecureAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getSecureAlgorithmRegex()` instead.
+ */
+deprecated string algorithmWhitelistRegex() { result = getSecureAlgorithmRegex() }

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

@@ -21,7 +21,7 @@ private class ShortStringLiteral extends StringLiteral {
 
 class BrokenAlgoLiteral extends ShortStringLiteral {
   BrokenAlgoLiteral() {
-    getValue().regexpMatch(algorithmBlacklistRegex()) and
+    getValue().regexpMatch(getInsecureAlgorithmRegex()) and
     // Exclude German and French sentences.
     not getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
   }

java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql

@@ -25,9 +25,9 @@ class InsecureAlgoLiteral extends ShortStringLiteral {
     // Algorithm identifiers should be at least two characters.
     getValue().length() > 1 and
     exists(string s | s = getLiteral() |
-      not s.regexpMatch(algorithmWhitelistRegex()) and
+      not s.regexpMatch(getSecureAlgorithmRegex()) and
       // Exclude results covered by another query.
-      not s.regexpMatch(algorithmBlacklistRegex())
+      not s.regexpMatch(getInsecureAlgorithmRegex())
     )
   }
 }

java/ql/src/semmle/code/java/security/Encryption.qll

@@ -1,3 +1,7 @@
+/**
+ * Provides predicates and classes relating to encryption in Java.
+ */
+
 import java
 
 class SSLClass extends RefType {
@@ -85,41 +89,51 @@ private string algorithmRegex(string algorithmString) {
       "((^|.*[A-Z]{2}|.*[^a-zA-Z])(" + algorithmString.toLowerCase() + ")([^a-z].*|$))"
 }
 
-/** Gets a blacklist of algorithms that are known to be insecure. */
-private string algorithmBlacklist() {
+/**
+ * Gets the name of an algorithm that is known to be insecure.
+ */
+string getAnInsecureAlgorithmName() {
   result = "DES" or
   result = "RC2" or
   result = "RC4" or
   result = "RC5" or
   result = "ARCFOUR" // a variant of RC4
 }
 
-// These are only bad if they're being used for encryption.
-private string hashAlgorithmBlacklist() {
+/**
+ * Gets the name of a hash algorithm that is insecure if it is being used for
+ * encryption.
+ */
+string getAnInsecureHashAlgorithmName() {
   result = "SHA1" or
   result = "MD5"
 }
 
-private string rankedAlgorithmBlacklist(int i) {
+private string rankedInsecureAlgorithm(int i) {
   // In this case we know these are being used for encryption, so we want to match
   // weak hash algorithms too.
-  result = rank[i](string s | s = algorithmBlacklist() or s = hashAlgorithmBlacklist())
+  result =
+    rank[i](string s | s = getAnInsecureAlgorithmName() or s = getAnInsecureHashAlgorithmName())
 }
 
-private string algorithmBlacklistString(int i) {
-  i = 1 and result = rankedAlgorithmBlacklist(i)
+private string insecureAlgorithmString(int i) {
+  i = 1 and result = rankedInsecureAlgorithm(i)
   or
-  result = rankedAlgorithmBlacklist(i) + "|" + algorithmBlacklistString(i - 1)
+  result = rankedInsecureAlgorithm(i) + "|" + insecureAlgorithmString(i - 1)
 }
 
-/** Gets a regex for matching strings that look like they contain a blacklisted algorithm. */
-string algorithmBlacklistRegex() {
-  result =
-    algorithmRegex(algorithmBlacklistString(max(int i | exists(rankedAlgorithmBlacklist(i)))))
+/**
+ * Gets the regular expression used for matching strings that look like they
+ * contain an algorithm that is known to be insecure.
+ */
+string getInsecureAlgorithmRegex() {
+  result = algorithmRegex(insecureAlgorithmString(max(int i | exists(rankedInsecureAlgorithm(i)))))
 }
 
-/** Gets a whitelist of algorithms that are known to be secure. */
-private string algorithmWhitelist() {
+/**
+ * Gets the name of an algorithm that is known to be secure.
+ */
+string getASecureAlgorithmName() {
   result = "RSA" or
   result = "SHA256" or
   result = "SHA512" or
@@ -130,20 +144,50 @@ private string algorithmWhitelist() {
   result = "ECIES"
 }
 
-private string rankedAlgorithmWhitelist(int i) { result = rank[i](algorithmWhitelist()) }
+private string rankedSecureAlgorithm(int i) { result = rank[i](getASecureAlgorithmName()) }
 
-private string algorithmWhitelistString(int i) {
-  i = 1 and result = rankedAlgorithmWhitelist(i)
+private string secureAlgorithmString(int i) {
+  i = 1 and result = rankedSecureAlgorithm(i)
   or
-  result = rankedAlgorithmWhitelist(i) + "|" + algorithmWhitelistString(i - 1)
+  result = rankedSecureAlgorithm(i) + "|" + secureAlgorithmString(i - 1)
 }
 
-/** Gets a regex for matching strings that look like they contain a whitelisted algorithm. */
-string algorithmWhitelistRegex() {
-  result =
-    algorithmRegex(algorithmWhitelistString(max(int i | exists(rankedAlgorithmWhitelist(i)))))
+/**
+ * Gets a regular expression for matching strings that look like they
+ * contain an algorithm that is known to be secure.
+ */
+string getSecureAlgorithmRegex() {
+  result = algorithmRegex(secureAlgorithmString(max(int i | exists(rankedSecureAlgorithm(i)))))
 }
 
+/**
+ * DEPRECATED: Terminology has been updated. Use `getAnInsecureAlgorithmName()`
+ * instead.
+ */
+deprecated string algorithmBlacklist() { result = getAnInsecureAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use
+ * `getAnInsecureHashAlgorithmName()` instead.
+ */
+deprecated string hashAlgorithmBlacklist() { result = getAnInsecureHashAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getInsecureAlgorithmRegex()` instead.
+ */
+deprecated string algorithmBlacklistRegex() { result = getInsecureAlgorithmRegex() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getASecureAlgorithmName()`
+ * instead.
+ */
+deprecated string algorithmWhitelist() { result = getASecureAlgorithmName() }
+
+/**
+ * DEPRECATED: Terminology has been updated. Use `getSecureAlgorithmRegex()` instead.
+ */
+deprecated string algorithmWhitelistRegex() { result = getSecureAlgorithmRegex() }
+
 /**
  * Any use of a cryptographic element that specifies an encryption
  * algorithm. For example, methods returning ciphers, decryption methods,

java/ql/test/library-tests/Encryption/blacklist.expected renamed to java/ql/test/library-tests/Encryption/insecure.expected

@@ -2,5 +2,5 @@ import default
 import semmle.code.java.security.Encryption
 
 from StringLiteral s
-where s.getLiteral().regexpMatch(algorithmWhitelistRegex())
+where s.getLiteral().regexpMatch(getInsecureAlgorithmRegex())
 select s

java/ql/test/library-tests/Encryption/whitelist.ql renamed to java/ql/test/library-tests/Encryption/insecure.ql

@@ -2,5 +2,5 @@ import default
 import semmle.code.java.security.Encryption
 
 from StringLiteral s
-where s.getLiteral().regexpMatch(algorithmBlacklistRegex())
+where s.getLiteral().regexpMatch(getSecureAlgorithmRegex())
 select s